From 66b60e9ac2ef9c12d22dcc31201e22e6eeb4804d Mon Sep 17 00:00:00 2001 From: Tobias Markmann 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 #include #include -#ifdef SWIFTEN_CACHE_JID_PREP -#include -#include -#endif - #include #include @@ -28,15 +21,6 @@ using namespace Swift; -#ifdef SWIFTEN_CACHE_JID_PREP -typedef std::unordered_map PrepCache; - -static std::mutex namePrepCacheMutex; -static PrepCache nodePrepCache; -static PrepCache domainPrepCache; -static PrepCache resourcePrepCache; -#endif - static const std::vector 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 lock(namePrepCacheMutex); - - std::pair 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