Summary: | Would be faster to not use a thread specific to implement StringImpl::empty() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Barraclough <barraclough> | ||||||
Component: | Platform | Assignee: | Gavin Barraclough <barraclough> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Gavin Barraclough
2010-03-10 15:29:42 PST
Created attachment 50447 [details]
The patch
~0% performance impact in JSC (possible minor progression from moving empty() from .h to .cpp).
~2% progression on Dromaeo DOM core & JS lib tests.
Comment on attachment 50447 [details]
The patch
r=me assuming you ran all the regression tests
Comment on attachment 50447 [details]
The patch
Sorry -- I said r- on IRC because DEFINE_STATIC_LOCAL is not threadsafe. The solution we agreed upon was to add a call to empty() at startup, to ensure that initialization of the shared empty static occurs before any JSC or WebCore threads are created.
(In reply to comment #3) > Sorry -- I said r- on IRC because DEFINE_STATIC_LOCAL is not threadsafe. The > solution we agreed upon was to add a call to empty() at startup, to ensure that > initialization of the shared empty static occurs before any JSC or WebCore > threads are created. WebCore has “startup” but I’m not sure JavaScriptCore does. Created attachment 50456 [details]
The patch
Added call to UStringImpl::empty() from initializeThreadingOnce() to ensure empty() is called in a threadsafe fashion the first time it is used.
Added call to StringImpl::empty() from the ThreadGlobalData constructor, to ensure empty() has been called from the main thread before any further threads are created.
Implemented fix as discussed with geoff on IRC, does anyone know a better startup hook I should be using in WebCore? Comment on attachment 50456 [details]
The patch
r=me
|