From 66b60e9ac2ef9c12d22dcc31201e22e6eeb4804d Mon Sep 17 00:00:00 2001
From: Tobias Markmann <tm@ayena.de>
Date: Wed, 10 Oct 2018 10:49:02 +0200
Subject: Remove JID prep caching from JID class

Ran a flamegraph diff on Swift login to an account with a
moderate number of JIDs in roster and more than 10 rooms
in auto join. It showed a difference less that 0.1% on the
changed method. As it does not make much of a difference for
Swift at least, caching is removed.

Test-Information:

Unit tests passes, Swift still logins fine on macOS.

Change-Id: Id33d6d1a655580e06e1364df717fd6f34cce5327

diff --git a/Swiften/JID/JID.cpp b/Swiften/JID/JID.cpp
index e8a4700..a31c19f 100644
--- a/Swiften/JID/JID.cpp
+++ b/Swiften/JID/JID.cpp
@@ -1,20 +1,13 @@
 /*
- * Copyright (c) 2010-2016 Isode Limited.
+ * Copyright (c) 2010-2018 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
-#define SWIFTEN_CACHE_JID_PREP
-
 #include <sstream>
 #include <string>
 #include <vector>
 
-#ifdef SWIFTEN_CACHE_JID_PREP
-#include <mutex>
-#include <unordered_map>
-#endif
-
 #include <boost/optional.hpp>
 
 #include <Swiften/Base/String.h>
@@ -28,15 +21,6 @@
 
 using namespace Swift;
 
-#ifdef SWIFTEN_CACHE_JID_PREP
-typedef std::unordered_map<std::string, std::string> PrepCache;
-
-static std::mutex namePrepCacheMutex;
-static PrepCache nodePrepCache;
-static PrepCache domainPrepCache;
-static PrepCache resourcePrepCache;
-#endif
-
 static const std::vector<char> escapedChars = {' ', '"', '&', '\'', '/', '<', '>', '@', ':'};
 
 static IDNConverter* idnConverter = nullptr;
@@ -124,54 +108,15 @@ void JID::nameprepAndSetComponents(const std::string& node, const std::string& d
         valid_ = false;
         return;
     }
-#ifndef SWIFTEN_CACHE_JID_PREP
-    node_ = idnConverter->getStringPrepared(node, IDNConverter::XMPPNodePrep);
-    domain_ = idnConverter->getStringPrepared(domain, IDNConverter::NamePrep);
-    resource_ = idnConverter->getStringPrepared(resource, IDNConverter::XMPPResourcePrep);
-#else
-    std::unique_lock<std::mutex> lock(namePrepCacheMutex);
-
-    std::pair<PrepCache::iterator, bool> r;
-
-    r = nodePrepCache.insert(std::make_pair(node, std::string()));
-    if (r.second) {
-        try {
-            r.first->second = idnConverter->getStringPrepared(node, IDNConverter::XMPPNodePrep);
-        }
-        catch (...) {
-            nodePrepCache.erase(r.first);
-            valid_ = false;
-            return;
-        }
-    }
-    node_ = r.first->second;
-
-    r = domainPrepCache.insert(std::make_pair(domain, std::string()));
-    if (r.second) {
-        try {
-            r.first->second = idnConverter->getStringPrepared(domain, IDNConverter::NamePrep);
-        }
-        catch (...) {
-            domainPrepCache.erase(r.first);
-            valid_ = false;
-            return;
-        }
-    }
-    domain_ = r.first->second;
 
-    r = resourcePrepCache.insert(std::make_pair(resource, std::string()));
-    if (r.second) {
-        try {
-            r.first->second = idnConverter->getStringPrepared(resource, IDNConverter::XMPPResourcePrep);
-        }
-        catch (...) {
-            resourcePrepCache.erase(r.first);
-            valid_ = false;
-            return;
-        }
+    try {
+        node_ = idnConverter->getStringPrepared(node, IDNConverter::XMPPNodePrep);
+        domain_ = idnConverter->getStringPrepared(domain, IDNConverter::NamePrep);
+        resource_ = idnConverter->getStringPrepared(resource, IDNConverter::XMPPResourcePrep);
+    } catch (...) {
+        valid_ = false;
+        return;
     }
-    resource_ = r.first->second;
-#endif
 
     if (domain_.empty()) {
         valid_ = false;
diff --git a/Swiften/JID/UnitTest/JIDTest.cpp b/Swiften/JID/UnitTest/JIDTest.cpp
index 0101a4f..aefda33 100644
--- a/Swiften/JID/UnitTest/JIDTest.cpp
+++ b/Swiften/JID/UnitTest/JIDTest.cpp
@@ -64,6 +64,7 @@ class JIDTest : public CppUnit::TestFixture
         CPPUNIT_TEST(testGetEscapedNode_BackslashAtEnd);
         CPPUNIT_TEST(testGetUnescapedNode);
         CPPUNIT_TEST(testGetUnescapedNode_XEP106Examples);
+        CPPUNIT_TEST(testStringPrepFailures);
         CPPUNIT_TEST_SUITE_END();
 
     public:
@@ -159,6 +160,12 @@ class JIDTest : public CppUnit::TestFixture
             CPPUNIT_ASSERT(!testling.isValid());
         }
 
+        void testStringPrepFailures() {
+            CPPUNIT_ASSERT_EQUAL(false, JID("foo@bar", "example.com").isValid());
+            CPPUNIT_ASSERT_EQUAL(false, JID("foo^", "example*com").isValid());
+            CPPUNIT_ASSERT_EQUAL(false, JID("foobar", "example^com").isValid());
+        }
+
         void testConstructorWithString_EmptyDomainWithResource() {
             JID testling("bar@/resource");
 
-- 
cgit v0.10.2-6-g49f6