StringImpl refcounting should be atomic - #1713
Open
justinmichaud wants to merge 1 commit into
Open
Conversation
justinmichaud
force-pushed
the
cherry-pick-atomic-ref
branch
2 times, most recently
from
August 14, 2026 20:17
f512a9d to
83732ab
Compare
https://bugs.webkit.org/show_bug.cgi?id=289962 rdar://147313174 Backport notes: I removed some optimizations; ref churn is not great, but still correct. Original notes: Reviewed by Antti Koivisto, Yusuke Suzuki, Chris Dumez, and Keith Miller. This patch takes the first step to making WTF::String thread-safe: Making m_refcount increment and decrement atomic. (Hash, flags, and the AtomString table are not yet atomic. I plan to address those in my next patch.) The direct code change is pretty small, but it measured as a performance regression. So most of this code is optimization. I got rid of about 1/3 of WTF::StringImpl refcounts on Speedometer and about 1/2 on JetStream. ARM has a special ldadd instruction for fast atomics. Net net, it measures as about 1.5X slower than just add. That's pretty good! But it still required some optimizations to be affordable. Intel atomics are much slower, so it took far more optimizations to get Intel across the finish line. A/B testers suggest that we might still have a < 0.5% JetStream regression on Intel. I checked in with Yusuke, Keith, and Maciej, and we can move forward because we have a large total JetStream speedup on Intel this year, so customers on older hardware will still see a net speedup when they get new software.
justinmichaud
force-pushed
the
cherry-pick-atomic-ref
branch
from
August 14, 2026 20:17
83732ab to
a63911f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There are a number of crashes related to this missing backport on 2.38. This leads to crashes in ~StringImpl, bmalloc, and a few other unexpected places in JSC coming from shared stringimpls in random places like gstreamer.
This backport is missing lots of optimizations from the ToT verision to reduce risk, so it is probably a larger perf regression. These should just avoid decreasing ref churn, not affect correctness.
a63911f