Title: [205995] trunk/Source/WTF
- Revision
- 205995
- Author
- [email protected]
- Date
- 2016-09-15 13:43:58 -0700 (Thu, 15 Sep 2016)
Log Message
TextBreakIterator: unconvolute character break cache
https://bugs.webkit.org/show_bug.cgi?id=162001
Reviewed by Michael Saboff.
Simplify the one-element cache.
Added here (fixed a bit after): https://bugs.webkit.org/attachment.cgi?id=144118&action=""
Updated to never use a lock, and always use weak cmpxchg here: https://bugs.webkit.org/attachment.cgi?id=261719&action=""
It's just trying to reduce the number of times it calls into ICU
to initialize a UBRK_CHARACTER. The implementation keeps a
one-element cache of the latest used one, which threads can
optimistically grab. Failure is fine (just create a new one), same
for failure to cache (just destroy it), but the logic is odd and
you technically need release / acquire semantics because the
UBRK_CHARACTER creation's store need to be visible on acquisition
(but realistically it was created and then used and *then* cached,
so it's probably been long ago enough that read reorders never
manifested).
Using exchange directly achieves this without the headache.
* wtf/text/TextBreakIterator.cpp:
(WTF::getNonSharedCharacterBreakIterator):
(WTF::cacheNonSharedCharacterBreakIterator):
(WTF::NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator):
(WTF::NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator):
(WTF::compareAndSwapNonSharedCharacterBreakIterator): Deleted.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (205994 => 205995)
--- trunk/Source/WTF/ChangeLog 2016-09-15 20:41:42 UTC (rev 205994)
+++ trunk/Source/WTF/ChangeLog 2016-09-15 20:43:58 UTC (rev 205995)
@@ -1,3 +1,35 @@
+2016-09-15 JF Bastien <[email protected]>
+
+ TextBreakIterator: unconvolute character break cache
+ https://bugs.webkit.org/show_bug.cgi?id=162001
+
+ Reviewed by Michael Saboff.
+
+ Simplify the one-element cache.
+
+ Added here (fixed a bit after): https://bugs.webkit.org/attachment.cgi?id=144118&action=""
+ Updated to never use a lock, and always use weak cmpxchg here: https://bugs.webkit.org/attachment.cgi?id=261719&action=""
+
+ It's just trying to reduce the number of times it calls into ICU
+ to initialize a UBRK_CHARACTER. The implementation keeps a
+ one-element cache of the latest used one, which threads can
+ optimistically grab. Failure is fine (just create a new one), same
+ for failure to cache (just destroy it), but the logic is odd and
+ you technically need release / acquire semantics because the
+ UBRK_CHARACTER creation's store need to be visible on acquisition
+ (but realistically it was created and then used and *then* cached,
+ so it's probably been long ago enough that read reorders never
+ manifested).
+
+ Using exchange directly achieves this without the headache.
+
+ * wtf/text/TextBreakIterator.cpp:
+ (WTF::getNonSharedCharacterBreakIterator):
+ (WTF::cacheNonSharedCharacterBreakIterator):
+ (WTF::NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator):
+ (WTF::NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator):
+ (WTF::compareAndSwapNonSharedCharacterBreakIterator): Deleted.
+
2016-09-15 Keith Miller <[email protected]>
Pragma out undefined-var-template warnings in JSC for JSObjects that are templatized
Modified: trunk/Source/WTF/wtf/text/TextBreakIterator.cpp (205994 => 205995)
--- trunk/Source/WTF/wtf/text/TextBreakIterator.cpp 2016-09-15 20:41:42 UTC (rev 205994)
+++ trunk/Source/WTF/wtf/text/TextBreakIterator.cpp 2016-09-15 20:43:58 UTC (rev 205995)
@@ -25,8 +25,8 @@
#include "LineBreakIteratorPoolICU.h"
#include "UTextProviderLatin1.h"
#include "UTextProviderUTF16.h"
+#include <atomic>
#include <mutex>
-#include <wtf/Atomics.h>
#include <wtf/text/StringView.h>
// FIXME: This needs a better name
@@ -813,30 +813,31 @@
iterator = nullptr;
}
-static TextBreakIterator* nonSharedCharacterBreakIterator;
+static std::atomic<TextBreakIterator*> nonSharedCharacterBreakIterator = ATOMIC_VAR_INIT(nullptr);
-static inline bool compareAndSwapNonSharedCharacterBreakIterator(TextBreakIterator* expected, TextBreakIterator* newValue)
+static inline TextBreakIterator* getNonSharedCharacterBreakIterator()
{
- return WTF::weakCompareAndSwap(&nonSharedCharacterBreakIterator, expected, newValue);
+ if (auto *res = nonSharedCharacterBreakIterator.exchange(nullptr, std::memory_order_acquire))
+ return res;
+ return initializeIterator(UBRK_CHARACTER);
}
+static inline void cacheNonSharedCharacterBreakIterator(TextBreakIterator* cacheMe)
+{
+ if (auto *old = nonSharedCharacterBreakIterator.exchange(cacheMe, std::memory_order_release))
+ ubrk_close(reinterpret_cast<UBreakIterator*>(old));
+}
+
NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator(StringView string)
{
- m_iterator = nonSharedCharacterBreakIterator;
-
- bool createdIterator = m_iterator && compareAndSwapNonSharedCharacterBreakIterator(m_iterator, nullptr);
- if (!createdIterator)
- m_iterator = initializeIterator(UBRK_CHARACTER);
- if (!m_iterator)
- return;
-
- m_iterator = setTextForIterator(*m_iterator, string);
+ if ((m_iterator = getNonSharedCharacterBreakIterator()))
+ m_iterator = setTextForIterator(*m_iterator, string);
}
NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator()
{
- if (!compareAndSwapNonSharedCharacterBreakIterator(0, m_iterator))
- ubrk_close(reinterpret_cast<UBreakIterator*>(m_iterator));
+ if (m_iterator)
+ cacheNonSharedCharacterBreakIterator(m_iterator);
}
NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator(NonSharedCharacterBreakIterator&& other)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
