PSA: Making WTF::String and WTF::AtomicString thread-safe

[Albert Wong (王重傑)]

Hello blink-dev,

After about 14 months of work, we've come up with a path to make Blink Strings thread-safe while also maintaining rough performance parity.**

Here is a basic roadmap/design doc. There is a very long thread on platform-archecture-dev that tracked all the incremental progress. Highlights below:

What made this hard:

• Making StringImpl::ref_count_ an std::atomic<> has a large perf impact on a number of hot-paths (eg Element::getAttribute()) due to excessive incrementing/decrementing of the refcount. Some of these caused numerous > 20% slowdowns on blink_perf benchmarks.
• Switching to atomic ops causes a 67k, largely unreducible bloat in Android binary size.
• The interaction with AtomicStringTable meant the delete operation on deference required some fairly tricky synchronization to handle lifetimes correctly.

What's the path forward:

• Land a set of changes to V8 bindings, AtomicStringTable, etc., that allows for lazy construction of Blink Strings from V8StringResource and/or lookups in Blink data structures w/o first creating a Blink String.  This mitigates most of the perf effects in the micro-benchmarks and in a few cases, yields a major perf benefit (see 26.6% improvement in AttributeDescenentSelector)
• Add bulk creation functions to StringImpl to reduce startup cost of static string, and construction of QualifiedNames.
• Make the thread-safety changes to AtomicStringTable and StringImpl.

I estimate it's likely a month of work to land the pieces of the proof-of-concept CL in a safe and clean way.

To see the latest full set of benchmarks, please look at column Z of the linux-perf tab here.  (Sadly due to the WebComponents deprecation many of the earlier benchmarks no longer render, but column Z will.)

Thanks to everyone who's been in on this journey! Special call outs to jbroman@ for all the code reviews and consults, haraken@ for keeping us focused on the right questions + decision making process, danakj@/dcheng@/mlippautz@/Richard....@arm.com for the low-level thread-safety and perf consults, smcgruer@ for framing the impact of making things threadsafe, agrieve@ for dealing with the android binary size sadness, dberris@ + the pinpoint team for fixing story tags so I could collect the requisite data, and all the other folks who have popped on to #bubblesort-chat or fielded odd questions at random hours.

Feel free to respond to this thread or ping me if there are questions! I'll start trying to land code next week.

-Albert

** How did this start? It was a super jet-lagged chat between carlosk, dcheng, jianli, tbansal and I in the Toronto Blinkon of Feb 2019. Yay for jet lag! Yay for Blinkon! Yay for Tim Hortons!

[Daniel Cheng]

Thanks! I'm very excited to see this close to becoming reality and not having to deal with random crashes caused by accidental multi-threaded access to String.

It'll also make it easier to explore extensions to projects like multiple Blink Isolates in the future. Yay!

Daniel

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CALcbsXDVjowc53xr-%3DZREybzOKVHD_NVE1NYPETO4UTY_i%3D1nA%40mail.gmail.com.

[Kentaro Hara]

Very excited 2 :D

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAF3XrKpALc1b5ct78SNR471ihXyS_NUv8EbZ7GYMaSjO_nk36w%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

[Jeremy Roman]

Looking forward to it! :)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jw8OUbJpR8WVC%2BYBMOG90BhQNnzeRU97vrOoL824MjPUw%40mail.gmail.com.

[Wez]

SGTM :D

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACuR13eW9_oB3TL77a9hr3iNfPu3pndvzDXuNS%2BdU7syq0Dr9w%40mail.gmail.com.