Bug 147154 – Fixed VM pool allocation should have a reserve for allocations that cannot fail

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

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Currently, if I run with a JIT pool that is artificially limited to 50KB, I get these failures due to crashes when we try to allocate things for which there is no fallback:

stress/lexical-let-loop-semantics.js.ftl-no-cjit-small-pool
stress/lexical-let-semantics.js.ftl-no-cjit-small-pool
stress/load-varargs-then-inlined-call-exit-in-foo.js.ftl-no-cjit-small-pool
stress/math-clz32-basics.js.ftl-no-cjit-small-pool
stress/multi-put-by-offset-multiple-transitions.js.ftl-no-cjit-small-pool
stress/multi-put-by-offset-reallocation-butterfly-cse.js.ftl-no-cjit-small-pool
stress/multi-put-by-offset-reallocation-cases.js.ftl-no-cjit-small-pool
stress/throw-from-ftl-call-ic-slow-path-cells.js.ftl-no-cjit-small-pool
stress/throw-from-ftl-call-ic-slow-path.js.ftl-no-cjit-small-pool
stress/type-of-functions-and-objects.js.ftl-no-cjit-small-pool
stress/typed-array-get-by-val-profiling.js.ftl-no-cjit-small-pool
stress/typed-array-put-by-val-profiling.js.ftl-no-cjit-small-pool
regress/script-tests/deltablue-varargs.js.ftl-no-cjit-small-pool
regress/script-tests/emscripten-cube2hash.js.ftl-no-cjit-small-pool
regress/script-tests/inlined-put-by-id-transition.js.ftl-no-cjit-small-pool
regress/script-tests/sorting-benchmark.js.ftl-no-cjit-small-pool
sunspider-1.0/date-format-tofte.js.ftl-no-cjit-small-pool
v8-v6/v8-crypto.js.ftl-no-cjit-small-pool
v8-v6/v8-deltablue.js.ftl-no-cjit-small-pool
v8-v6/v8-earley-boyer.js.ftl-no-cjit-small-pool
v8-v6/v8-raytrace.js.ftl-no-cjit-small-pool
v8-v6/v8-regexp.js.ftl-no-cjit-small-pool
v8-v6/v8-richards.js.ftl-no-cjit-small-pool
v8-v6/v8-splay.js.ftl-no-cjit-small-pool
stress/const-loop-semantics.js.ftl-no-cjit-small-pool
stress/const-tdz.js.ftl-no-cjit-small-pool
stress/constant-folding-osr-exit.js.ftl-no-cjit-small-pool
stress/flatten-oversize-dictionary-object.js.ftl-no-cjit-small-pool
stress/for-in-tests.js.ftl-no-cjit-small-pool

[Filip Pizlo]

Created attachment 257196 [details]
the patch

[WebKit Commit Bot]

Attachment 257196 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/Options.cpp:353:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Geoffrey Garen]

Comment on attachment 257196 [details]
the patch

r=me

[Filip Pizlo]

Created attachment 257203 [details]
performance results

This patch is neutral.  Using a 16MB limit is also neutral.  But this is cutting it close - a 1.6MB limit causes huge slow-downs.

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/187125

[Alexey Proskuryakov]

Looks like this broke a lot of tests:

https://build.webkit.org/builders/Apple%20Yosemite%20LLINT%20CLoop%20%28BuildAndTest%29/builds/7561/steps/webkit-jsc-cloop-test/logs/stdio

https://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/11551/steps/webkit-32bit-jsc-test/logs/stdio

https://build.webkit.org/builders/Apple%20Mavericks%20LLINT%20CLoop%20%28BuildAndTest%29/builds/16973/steps/webkit-jsc-cloop-test/logs/stdio

[Filip Pizlo]

(In reply to comment #7)
> Looks like this broke a lot of tests:
> 
> https://build.webkit.org/builders/
> Apple%20Yosemite%20LLINT%20CLoop%20%28BuildAndTest%29/builds/7561/steps/
> webkit-jsc-cloop-test/logs/stdio
> 
> https://build.webkit.org/builders/Apple%20Mavericks%2032-
> bit%20JSC%20%28BuildAndTest%29/builds/11551/steps/webkit-32bit-jsc-test/logs/
> stdio

This isn't real.  On the bot all tests fail.  I'm testing 32-bit locally and nothing fails.

> 
> https://build.webkit.org/builders/
> Apple%20Mavericks%20LLINT%20CLoop%20%28BuildAndTest%29/builds/16973/steps/
> webkit-jsc-cloop-test/logs/stdio

[Filip Pizlo]

(In reply to comment #7)
> Looks like this broke a lot of tests:
> 
> https://build.webkit.org/builders/
> Apple%20Yosemite%20LLINT%20CLoop%20%28BuildAndTest%29/builds/7561/steps/
> webkit-jsc-cloop-test/logs/stdio

This is almost certainly a fluke.  This reports that all tests fail, but on my machine I am seeing nothing but passes so far.

> 
> https://build.webkit.org/builders/Apple%20Mavericks%2032-
> bit%20JSC%20%28BuildAndTest%29/builds/11551/steps/webkit-32bit-jsc-test/logs/
> stdio
> 
> https://build.webkit.org/builders/
> Apple%20Mavericks%20LLINT%20CLoop%20%28BuildAndTest%29/builds/16973/steps/
> webkit-jsc-cloop-test/logs/stdio

Ditto.

[Filip Pizlo]

(In reply to comment #9)
> (In reply to comment #7)
> > Looks like this broke a lot of tests:
> > 
> > https://build.webkit.org/builders/
> > Apple%20Yosemite%20LLINT%20CLoop%20%28BuildAndTest%29/builds/7561/steps/
> > webkit-jsc-cloop-test/logs/stdio
> 
> This is almost certainly a fluke.  This reports that all tests fail, but on
> my machine I am seeing nothing but passes so far.
> 
> > 
> > https://build.webkit.org/builders/Apple%20Mavericks%2032-
> > bit%20JSC%20%28BuildAndTest%29/builds/11551/steps/webkit-32bit-jsc-test/logs/
> > stdio
> > 
> > https://build.webkit.org/builders/
> > Apple%20Mavericks%20LLINT%20CLoop%20%28BuildAndTest%29/builds/16973/steps/
> > webkit-jsc-cloop-test/logs/stdio
> 
> Ditto.

I've now run all of the stress tests in cloop, and found some longstanding issues, but certainly nothing like what the bots were reporting.

https://bugs.webkit.org/show_bug.cgi?id=147167
https://bugs.webkit.org/show_bug.cgi?id=147168

[Filip Pizlo]

Well, looks like the bots were right.  This was a timing issue that only manifested on some debug builds.  The fix is:

http://trac.webkit.org/changeset/187139

[Csaba Osztrogonác]

(In reply to comment #11)
> Well, looks like the bots were right.  This was a timing issue that only
> manifested on some debug builds.  The fix is:
> 
> http://trac.webkit.org/changeset/187139

The Apple Yosemite Debug bot still has 914 asserting tests:
https://build.webkit.org/builders/Apple%20Yosemite%20Debug%20JSC%20%28Tests%29

[Filip Pizlo]

(In reply to comment #12)
> (In reply to comment #11)
> > Well, looks like the bots were right.  This was a timing issue that only
> > manifested on some debug builds.  The fix is:
> > 
> > http://trac.webkit.org/changeset/187139
> 
> The Apple Yosemite Debug bot still has 914 asserting tests:
> https://build.webkit.org/builders/
> Apple%20Yosemite%20Debug%20JSC%20%28Tests%29

Ooops, fix on the way.

[Filip Pizlo]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Well, looks like the bots were right.  This was a timing issue that only
> > > manifested on some debug builds.  The fix is:
> > > 
> > > http://trac.webkit.org/changeset/187139
> > 
> > The Apple Yosemite Debug bot still has 914 asserting tests:
> > https://build.webkit.org/builders/
> > Apple%20Yosemite%20Debug%20JSC%20%28Tests%29
> 
> Ooops, fix on the way.

Fixed: http://trac.webkit.org/changeset/187175