Skip to content

Commit

Permalink
TextBreakIteratorCache shouldn't be used from worker threads.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260498
<rdar://113654067>

Reviewed by David Kilzer.

An alternative here would be to have a cache per-thread, but I'm not aware of an easy way to
store thread local data from WTF currently.

This just skips the cache for non-main threads, and we can consider adding it back as needed
for performance in the future.

* Source/WTF/wtf/text/TextBreakIterator.h:
(WTF::CachedTextBreakIterator::CachedTextBreakIterator):
(WTF::CachedTextBreakIterator::~CachedTextBreakIterator):

Canonical link: https://commits.webkit.org/267175@main
  • Loading branch information
mattwoodrow committed Aug 23, 2023
1 parent c324070 commit 3e4dfc2
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions Source/WTF/wtf/text/TextBreakIterator.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ class TextBreakIterator {
}

private:
friend class CachedTextBreakIterator;
friend class TextBreakIteratorCache;

using Backing = std::variant<TextBreakIteratorICU, TextBreakIteratorPlatform>;
Expand Down Expand Up @@ -151,6 +152,7 @@ class TextBreakIteratorCache {

TextBreakIterator take(StringView string, const UChar* priorContext, unsigned priorContextLength, TextBreakIterator::Mode mode, TextBreakIterator::ContentAnalysis contentAnalysis, const AtomString& locale)
{
ASSERT(isMainThread());
auto iter = std::find_if(m_unused.begin(), m_unused.end(), [&](TextBreakIterator& candidate) {
return candidate.mode() == mode && candidate.contentAnalysis() == contentAnalysis && candidate.locale() == locale;
});
Expand All @@ -164,6 +166,7 @@ class TextBreakIteratorCache {

void put(TextBreakIterator&& iterator)
{
ASSERT(isMainThread());
m_unused.append(WTFMove(iterator));
if (m_unused.size() > capacity)
m_unused.remove(0);
Expand All @@ -181,13 +184,13 @@ class CachedTextBreakIterator {
WTF_MAKE_FAST_ALLOCATED;
public:
CachedTextBreakIterator(StringView string, const UChar* priorContext, unsigned priorContextLength, TextBreakIterator::Mode mode, const AtomString& locale, TextBreakIterator::ContentAnalysis contentAnalysis = TextBreakIterator::ContentAnalysis::Mechanical)
: m_backing(TextBreakIteratorCache::singleton().take(string, priorContext, priorContextLength, mode, contentAnalysis, locale))
: m_backing(isMainThread() ? TextBreakIteratorCache::singleton().take(string, priorContext, priorContextLength, mode, contentAnalysis, locale) : TextBreakIterator(string, priorContext, priorContextLength, mode, contentAnalysis, locale))
{
}

~CachedTextBreakIterator()
{
if (m_backing)
if (m_backing && isMainThread())
TextBreakIteratorCache::singleton().put(WTFMove(*m_backing));
}

Expand Down

0 comments on commit 3e4dfc2

Please sign in to comment.