Bug 141221

Summary: Remove concept of makeUsableFromMultipleThreads()
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, mmirman, msaboff, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Perf number for the patch.
none
the patch. mhahnenb: review+

Description Mark Lam 2015-02-03 17:08:57 PST
Currently, we rely on VM::makeUsableFromMultipleThreads() being called before we start entering the VM (which means acquiring the JSLock) from different threads.  Acquisition of the JSLock will register the acquiring thread to the VM's thread registry if not added.  However, it will only do this if the VM's thread specific key has been initialized by makeUsableFromMultipleThreads().

This is fragile and also does not read intuitively because one would expect to acquire the JSLock before calling any methods on the VM.  This exactly what JSGlobalContextCreateInGroup() did (acquire the lock before calling makeUsableFromMultipleThreads()), but is wrong.  The result is that the invoking thread will not have been registered with the VM during that first entry into the VM.

The fix is to make it so that we initialize the VM's thread specific key on construction of the VM's MachineThreads registry instead of relying on makeUsableFromMultipleThreads() being called.  With this, we can eliminate makeUsableFromMultipleThreads() altogether.
Comment 1 Radar WebKit Bug Importer 2015-02-03 17:10:15 PST
<rdar://problem/19709137>
Comment 2 Mark Lam 2015-02-03 17:20:11 PST
Created attachment 245992 [details]
Perf number for the patch.

Perf numbers are neutral on aggregate.  There are some benchmark components that claim to have a significant difference, but I think that may be due to noise:

SunSpider:
   bitops-bitwise-and                                 3.7012+-0.0984     ^      3.4934+-0.1020        ^ definitely 1.0595x faster

Kraken:
   json-stringify-tinderbox                          141.180+-0.947      ^     127.461+-0.712         ^ definitely 1.1076x faster

JSRegress:
   string-char-code-at                               34.9297+-0.0626     !     35.3132+-0.2146        ! definitely 1.0110x slower
   string-repeat-arith                               83.8087+-5.9984     ^     73.9739+-1.4799        ^ definitely 1.1329x faster

I sincerely doubt that this patch could have contributed those gains.
Comment 3 Mark Lam 2015-02-03 17:32:14 PST
Created attachment 245996 [details]
the patch.
Comment 4 Mark Hahnenberg 2015-02-04 07:40:12 PST
Comment on attachment 245996 [details]
the patch.

Nice. R=me.
Comment 5 Mark Lam 2015-02-04 10:02:34 PST
Thanks for the review.  Landed in r179609: <http://trac.webkit.org/r179609>.