Bug 159754 – Repeatedly creating and destroying workers that enqueue DFG plans can outpace the DFG worklist, which then causes VM shutdown to stall, which then causes memory growth

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

Patch forthcoming

[Filip Pizlo]

Created attachment 283597 [details]
the change is small but the test is huge

[Build Bot]

Comment on attachment 283597 [details]
the change is small but the test is huge

Attachment 283597 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1678247

New failing tests:
workers/bomb.html
webgl/1.0.2/conformance/typedarrays/typed-arrays-in-workers.html
http/tests/fetch/caching-with-different-options.html

[Build Bot]

Created attachment 283600 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283597 [details]
the change is small but the test is huge

Attachment 283597 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1678308

New failing tests:
webgl/1.0.2/conformance/typedarrays/typed-arrays-in-workers.html
workers/bomb.html

[Build Bot]

Created attachment 283602 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283597 [details]
the change is small but the test is huge

Attachment 283597 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1678312

New failing tests:
workers/bomb.html
imported/w3c/web-platform-tests/streams/readable-streams/bad-underlying-sources.https.html
imported/w3c/web-platform-tests/XMLHttpRequest/open-user-password-non-same-origin.htm

[Build Bot]

Created attachment 283603 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.5

[Build Bot]

Comment on attachment 283597 [details]
the change is small but the test is huge

Attachment 283597 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1678319

New failing tests:
webgl/1.0.2/conformance/context/context-release-with-workers.html

[Build Bot]

Created attachment 283604 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Filip Pizlo]

Oh man, such a perfect score!

[Filip Pizlo]

Created attachment 283606 [details]
the patch

Should fix those crashes.  It was a latent bug!  We'd leave cancelled plans in m_readyPlans!

[Build Bot]

Comment on attachment 283606 [details]
the patch

Attachment 283606 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1678616

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 283613 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283606 [details]
the patch

Attachment 283606 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1678621

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 283616 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283606 [details]
the patch

Attachment 283606 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1678632

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 283618 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283606 [details]
the patch

Attachment 283606 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1678647

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 283619 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.5

[Filip Pizlo]

Created attachment 283622 [details]
the patch

[Build Bot]

Comment on attachment 283622 [details]
the patch

Attachment 283622 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1678923

New failing tests:
workers/bomb.html

[Build Bot]

Created attachment 283625 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Geoffrey Garen]

Comment on attachment 283622 [details]
the patch

r=me if you fix the bot failure.

Regressions: Unexpected timeouts (1)
  workers/bomb.html [ Timeout ]

Can we make this test less aggressive? Otherwise, I guess we have to skip it in debug builds.

[Filip Pizlo]

(In reply to comment #23)
> Comment on attachment 283622 [details]
> the patch
> 
> r=me if you fix the bot failure.
> 
> Regressions: Unexpected timeouts (1)
>   workers/bomb.html [ Timeout ]
> 
> Can we make this test less aggressive? Otherwise, I guess we have to skip it
> in debug builds.

I think we have to skip it in debug builds.  This test won't trigger the bug unless you run it for about as long as I'm running it for.

Alexey and I have been chatting about how to incorporate such tests.  I want this test to grow larger - for example it doesn't yet create V8-v6 workers - because that will catch more bugs.  Like, we might get some interesting worker-related GC bugs if we throw v8-splay into the mix.  That means that the test will probably run too long to make sense for run-webkit-tests even in release mode.  Probably we want this test to run on a bot, but we don't currently have a bot for long-running stress tests.  We will have to think about how best to do this.  I was thinking that in the short-term we might have to skip this test altogether, but leave it checked in.  We can run it manually from time to time.  As we accumulate more such tests, hopefully we'll think of a good process for them.

I bet we'll need more such tests once we do SharedArrayBuffer and concurrent JS.  I remember that I had a collection of regression tests for Fiji that didn't fit well into continuous integration because you had to run them for half an hour to be sure that everything was fine.  They all had to do with races between user-started threads.  The gnarliest races involved the VM's handling of thread start/stop, which is sort of what workers/bomb.html is testing.

[Filip Pizlo]

Created attachment 283670 [details]
patch for landing

[Filip Pizlo]

Created attachment 283673 [details]
patch for landing

[Filip Pizlo]

Created attachment 283674 [details]
performance

It's neutral.  I was worried it would hurt because I removed the Compiled state from Plan, but it seems that state was 100% useless.

[Filip Pizlo]

Landed in https://trac.webkit.org/changeset/203370

[Carlos Alberto Lopez Perez]

I have marked workers/bomb.html as slow for GTK/EFL in https://trac.webkit.org/changeset/203509 as it was timing out constantly. It takes 15-20 seconds to pass which is too much for standard tests.

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#tests=workers%2Fbomb.html

Maybe it should be marked as slow on the general test expectations file? Seems it also takes a lot on mac -- On a macpro that I have at hand here it takes 13 seconds.

[Claudio Saavedra]

Did the same for WPE, fwiw. http://trac.webkit.org/changeset/217539

[Michael Catanzaro]

It's timing out for GTK even when marked slow. We'll just need to skip it.