Remove the thread lock from API Shims if the VM is not a APIShared type. The similar logic could be found in old revision of JSLock, which does the lock or unlock only when exec->globalData().isSharedInstance() is true. By removing the thread lock, the JSC API static function call could be 60% times faster than before.
Created attachment 206479 [details] Test case
No cache Cache result(ms) 1. 8000 3190 2. 7600 3160 3. 7987 3208 4. 8002 3305 5. 8155 3173 6. 7539 3125 7. 8562 3135 8. 7455 3137 9. 7936 3122 10. 7461 3103 --------------------------------------- avg 7869 3166
The first row should be With lock Without lock (In reply to comment #2) > No cache Cache > result(ms) > 1. 8000 3190 > 2. 7600 3160 > 3. 7987 3208 > 4. 8002 3305 > 5. 8155 3173 > 6. 7539 3125 > 7. 8562 3135 > 8. 7455 3137 > 9. 7936 3122 > 10. 7461 3103 > --------------------------------------- > avg 7869 3166
Created attachment 206480 [details] proposal patch
Comment on attachment 206480 [details] proposal patch Even unshared VMs need locking, since they can be used on more than one thread. I'm intrigued by your test case, though. Maybe there's another way to optimize this.
(In reply to comment #5) > (From update of attachment 206480 [details]) > Even unshared VMs need locking, since they can be used on more than one thread. > > I'm intrigued by your test case, though. Maybe there's another way to optimize this. I opened a bug with the same test before and Yi is trying to resolve this issue through a series of patches. Your help is very appreciated. https://bugs.webkit.org/show_bug.cgi?id=97122
Thanks for review. How about removing the API thread lock/unlock for the VM which has an exclusive thread? For example, WK2 is using JSC APIs within the injected bundle, which runs in the WebCore's exclusive VM. I believe it is thread safety when executing JSC APIs in the WebCore's VM. (In reply to comment #5) > (From update of attachment 206480 [details]) > Even unshared VMs need locking, since they can be used on more than one thread. > > I'm intrigued by your test case, though. Maybe there's another way to optimize this.
> Thanks for review. How about removing the API thread lock/unlock for the VM which has an exclusive thread? For example, WK2 is using JSC APIs within the injected bundle, which runs in the WebCore's exclusive VM. I believe it is thread safety when executing JSC APIs in the WebCore's VM. I think a specialized design like that would need a more real-world benchmark to justify itself. There are lots of opportunities for optimizing JSLock.
I do have a more real-world test for this. Will add a patch for review and post a real-world benchmark next week. Thanks in advanced! (In reply to comment #8) > > Thanks for review. How about removing the API thread lock/unlock for the VM which has an exclusive thread? For example, WK2 is using JSC APIs within the injected bundle, which runs in the WebCore's exclusive VM. I believe it is thread safety when executing JSC APIs in the WebCore's VM. > > I think a specialized design like that would need a more real-world benchmark to justify itself. > > There are lots of opportunities for optimizing JSLock.
Created attachment 206576 [details] Remove the thread lock from API Shims if the VM has an exclusive thread
Created attachment 207361 [details] Test sample code for JSC API
Created attachment 207363 [details] Test result
The added test code is porting from JSTweener by using JSC APIs (WebKit2 injected bundle). http://coderepos.org/share/wiki/JSTweener The test is a rendering heavy page, so the PFS changes for tuning on/off thread lock may not be 'significant' - the average fps was increased from ~31 to ~34 on my machine after applied my patch. (I was hoping I can add a JS property accessing heavy page, but which turns out it needs a lot more work) In addition, I have run this test on my Mac for hundreds times, and no thread crash observed.
Comment on attachment 206576 [details] Remove the thread lock from API Shims if the VM has an exclusive thread r=me
Thanks for reviewing it :) (In reply to comment #14) > (From update of attachment 206576 [details]) > r=me
Comment on attachment 206576 [details] Remove the thread lock from API Shims if the VM has an exclusive thread Clearing flags on attachment: 206576 Committed r153331: <http://trac.webkit.org/changeset/153331>
All reviewed patches have been landed. Closing bug.