Bug 162001 - TextBreakIterator: unconvolute character break cache
Summary: TextBreakIterator: unconvolute character break cache
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: JF Bastien
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-14 17:11 PDT by JF Bastien
Modified: 2016-09-15 13:50 PDT (History)
7 users (show)

See Also:


Attachments
patch (4.79 KB, patch)
2016-09-14 17:16 PDT, JF Bastien
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2016-09-14 17:11:58 PDT
TextBreakIterator's character break cache is weird.

Added here (fixed a bit after): https://bugs.webkit.org/attachment.cgi?id=144118&action=prettypatch
Updated to never use a lock, and always use weak cmpxchg here: https://bugs.webkit.org/attachment.cgi?id=261719&action=prettypatch

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).
Comment 1 JF Bastien 2016-09-14 17:16:45 PDT
Created attachment 288901 [details]
patch

Note that this still leaks any cached instance on shutdown, but it didn't matter before so likely doesn't now.

It's also not a good fit for std::once_flag + std::call_once because IIUC multiple threads can't use the instance concurrently (the cache just passes it around, loaning it to *one* thread at a time).
Comment 2 Michael Saboff 2016-09-15 13:23:01 PDT
Comment on attachment 288901 [details]
patch

r=me
Comment 3 WebKit Commit Bot 2016-09-15 13:46:07 PDT
Comment on attachment 288901 [details]
patch

Clearing flags on attachment: 288901

Committed r205995: <http://trac.webkit.org/changeset/205995>
Comment 4 WebKit Commit Bot 2016-09-15 13:46:12 PDT
All reviewed patches have been landed.  Closing bug.