llint_slow_path_put_by_id has the following line of code: 599 ConcurrentJITLocker locker(codeBlock->m_lock); and later in the same scope: 618 StructureChain* chain = structure->prototypeChain(exec); which can cause a garbage collection. Garbage collection acquires the ConcurrentJITLock as it processes CodeBlocks, causing a deadlock.
(In reply to comment #0) > llint_slow_path_put_by_id has the following line of code: > > 599 ConcurrentJITLocker locker(codeBlock->m_lock); > > and later in the same scope: > > 618 StructureChain* chain = structure->prototypeChain(exec); > > which can cause a garbage collection. Garbage collection acquires the ConcurrentJITLock as it processes CodeBlocks, causing a deadlock. L
(In reply to comment #1) > (In reply to comment #0) > > llint_slow_path_put_by_id has the following line of code: > > > > 599 ConcurrentJITLocker locker(codeBlock->m_lock); > > > > and later in the same scope: > > > > 618 StructureChain* chain = structure->prototypeChain(exec); > > > > which can cause a garbage collection. Garbage collection acquires the ConcurrentJITLock as it processes CodeBlocks, causing a deadlock. > > L Dunno what happened there. I meant to say "Lol."
(In reply to comment #2) > (In reply to comment #1) > > (In reply to comment #0) > > > llint_slow_path_put_by_id has the following line of code: > > > > > > 599 ConcurrentJITLocker locker(codeBlock->m_lock); > > > > > > and later in the same scope: > > > > > > 618 StructureChain* chain = structure->prototypeChain(exec); > > > > > > which can cause a garbage collection. Garbage collection acquires the ConcurrentJITLock as it processes CodeBlocks, causing a deadlock. > > > > L > > Dunno what happened there. I meant to say "Lol." We could either move the call to prototypeChain(exec) up above the ConcurrentJITLocker, or we could do my favorite thing: DeferGC. I like DeferGC more since it's more comprehensive.
(In reply to comment #3) > (In reply to comment #2) > > (In reply to comment #1) > > > (In reply to comment #0) > > > > llint_slow_path_put_by_id has the following line of code: > > > > > > > > 599 ConcurrentJITLocker locker(codeBlock->m_lock); > > > > > > > > and later in the same scope: > > > > > > > > 618 StructureChain* chain = structure->prototypeChain(exec); > > > > > > > > which can cause a garbage collection. Garbage collection acquires the ConcurrentJITLock as it processes CodeBlocks, causing a deadlock. > > > > > > L > > > > Dunno what happened there. I meant to say "Lol." > > We could either move the call to prototypeChain(exec) up above the ConcurrentJITLocker, or we could do my favorite thing: DeferGC. > > I like DeferGC more since it's more comprehensive. Yeah I like that too.
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (In reply to comment #1) > > > > (In reply to comment #0) > > > > > llint_slow_path_put_by_id has the following line of code: > > > > > > > > > > 599 ConcurrentJITLocker locker(codeBlock->m_lock); > > > > > > > > > > and later in the same scope: > > > > > > > > > > 618 StructureChain* chain = structure->prototypeChain(exec); > > > > > > > > > > which can cause a garbage collection. Garbage collection acquires the ConcurrentJITLock as it processes CodeBlocks, causing a deadlock. > > > > > > > > L > > > > > > Dunno what happened there. I meant to say "Lol." > > > > We could either move the call to prototypeChain(exec) up above the ConcurrentJITLocker, or we could do my favorite thing: DeferGC. > > > > I like DeferGC more since it's more comprehensive. > > Yeah I like that too. We could even make ConcurrentJITLocker do an implicit DeferGC. Thoughts?
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (In reply to comment #2) > > > > (In reply to comment #1) > > > > > (In reply to comment #0) > > > > > > llint_slow_path_put_by_id has the following line of code: > > > > > > > > > > > > 599 ConcurrentJITLocker locker(codeBlock->m_lock); > > > > > > > > > > > > and later in the same scope: > > > > > > > > > > > > 618 StructureChain* chain = structure->prototypeChain(exec); > > > > > > > > > > > > which can cause a garbage collection. Garbage collection acquires the ConcurrentJITLock as it processes CodeBlocks, causing a deadlock. > > > > > > > > > > L > > > > > > > > Dunno what happened there. I meant to say "Lol." > > > > > > We could either move the call to prototypeChain(exec) up above the ConcurrentJITLocker, or we could do my favorite thing: DeferGC. > > > > > > I like DeferGC more since it's more comprehensive. > > > > Yeah I like that too. > > We could even make ConcurrentJITLocker do an implicit DeferGC. Thoughts? Or even better: have two versions of ConcurrentJITLocker. One version sets a thread-local variable that in causes all GC allocations and other GC calls to fail in debug mode. We could use that in as many places as possible. The cool thing about this is that it also guarantees that the method is safe to call from a concurrent thread, from the GC's standpoint. Another version does what you say. We can then try to use the asserting version in as many places as possible. Probably by doing this we will learn some fun surprises.
Created attachment 214030 [details] Patch
Comment on attachment 214030 [details] Patch love it.
Comment on attachment 214030 [details] Patch Attachment 214030 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/3950221
Created attachment 214031 [details] build fix
Comment on attachment 214031 [details] build fix Attachment 214031 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3776306
Comment on attachment 214031 [details] build fix Attachment 214031 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/3946253
Created attachment 214035 [details] omgjustbuild
Comment on attachment 214035 [details] omgjustbuild Rejecting attachment 214035 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 214035, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: avaScriptCore/llint/LLIntSlowPaths.cpp patching file Source/JavaScriptCore/runtime/ConcurrentJITLock.h patching file Source/JavaScriptCore/runtime/InitializeThreading.cpp patching file Source/JavaScriptCore/runtime/JSCellInlines.h patching file Source/JavaScriptCore/runtime/Structure.cpp patching file Source/JavaScriptCore/runtime/Structure.h Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Filip Pizlo']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.appspot.com/results/3766906
Committed r157413: <http://trac.webkit.org/changeset/157413>
Re-opened since this is blocked by bug 122779
This looks like the most likely culprit for WebGL and other crashes and timeouts: http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r157416%20(10770)/results.html http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r157419%20(13809)/results.html Rolled out in <https://trac.webkit.org/r157424>. We'll see if this helps.
This may have improved Dromaeo/jslib-event-jquery's score by ~28%: https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-lion%22%2C%22Dromaeo%2Fjslib-event-jquery%3ARuns%22%5D%2C%5B%22mac-mountainlion%22%2C%22Dromaeo%2Fjslib-event-jquery%3ARuns%22%5D%2C%5B%22mac-mountainlion%22%2C%22Dromaeo%2Fjslib-event-prototype%3ARuns%22%5D%2C%5B%22mac-lion%22%2C%22Dromaeo%2Fjslib-event-prototype%3ARuns%22%5D%5D&days=52&zoom=%5B1381585431357.5227%2C1381827392596%5D
(In reply to comment #18) > This may have improved Dromaeo/jslib-event-jquery's score by ~28%: > https://perf.webkit.org/#mode=charts&chartList=%5B%5B%22mac-lion%22%2C%22Dromaeo%2Fjslib-event-jquery%3ARuns%22%5D%2C%5B%22mac-mountainlion%22%2C%22Dromaeo%2Fjslib-event-jquery%3ARuns%22%5D%2C%5B%22mac-mountainlion%22%2C%22Dromaeo%2Fjslib-event-prototype%3ARuns%22%5D%2C%5B%22mac-lion%22%2C%22Dromaeo%2Fjslib-event-prototype%3ARuns%22%5D%5D&days=52&zoom=%5B1381585431357.5227%2C1381827392596%5D Uhh, I don't think so. The patch was rolled out, but we still have the perf win. Also, this patch just fixes a deadlock, no reason it would improve performance.
So it's probably either http://trac.webkit.org/changeset/157412 or http://trac.webkit.org/changeset/157411
(In reply to comment #20) > So it's probably either http://trac.webkit.org/changeset/157412 or http://trac.webkit.org/changeset/157411 Yeah, my money is on http://trac.webkit.org/changeset/157411.
Created attachment 214399 [details] Patch
Comment on attachment 214399 [details] Patch r=me based on previous reviews, assuming tests pass.
Committed r157539: <http://trac.webkit.org/changeset/157539>