The browser uses parallel compilation by default and the shell does not. Now that --enable-threadsafe is on by default for the shell, it makes sense to switch this to match the browser.

We should be careful with jit-tests. In particular, I think we should use --ion-parallel-compile=off for the --ion-eager runs we have with jit_test.py --tbpl because: (1) With --ion-eager we want to Ion-compile as much as possible. With parallel compilation it's possible for the script to finish before the background compilation is done and this does not give us a chance to enter Ion code. (2) We should still run tests without parallel compilation. This matters for workers and single core hardware.

https://tbpl.mozilla.org/?tree=Try&rev=89501ccefd37 This toggles the pref and updates --tbpl as requested. Since I was touching the flags anyway, I moved them to a common location and added support for --tbpl to the jsreftests harness. To pay for the new complexity I removed --jitflags: it appears to have been actively broken since we've removed jaegermonkey anyway, so I don't think anyone will mind. Jason, if you are sr+ for this, I will send a message out to js-internals before landing. Sorry I missed that last time!

Comment on attachment 822529 [details] [diff] [review] default_parallel_compile-v0.diff Review of attachment 822529 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, Terrence.

Comment on attachment 822529 [details] [diff] [review] default_parallel_compile-v0.diff Review of attachment 822529 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for doing this! ::: js/src/shell/js.cpp @@ +5566,5 @@ > if (op->getBoolOption("ion-compile-try-catch")) > jit::js_IonOptions.compileTryCatch = true; > > #ifdef JS_THREADSAFE > + bool parallelCompilation = true; This doesn't handle --ion-parallel-compile=off correctly. I think you want something like this: bool parallelCompilation = true; if (const char *str = op->getStringOption("ion-parallel-compile")) { if (strcmp(str, "off") == 0) parallelCompilation = false; else if (strcmp(str, "on") != 0) return OptionFailure("ion-parallel-compile", str); } And remove the "In shell builds, parallel compilation is only enabled with an explicit option." comment in this function. OffThreadIonCompilationEnabled also checks helperThreadCount() != 0 so I think we can safely remove that check here. r=me with that.

A month later and this is finally green. https://hg.mozilla.org/integration/mozilla-inbound/rev/c14453ff8764

And allow --ion-parallel-compile to be recognized on non-threadsafe builds so that SM(r) doesn't break. https://hg.mozilla.org/integration/mozilla-inbound/rev/c63eaabaefb1

I had to back this out due to frequent jit-test failures in the parallel tests. https://hg.mozilla.org/integration/mozilla-inbound/rev/20e4d6b3c819 https://tbpl.mozilla.org/php/getParsedLog.php?id=31514590&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=31516240&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=31516449&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=31516979&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=31515941&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=31517053&tree=Mozilla-Inbound

The failures seem to pertain to bailouts. Marking this bug as needinfo shu / pnkfelix / myself so that one of us can look into it shortly.

Surprise surprise, I can't reproduce this locally. However, reading through the logs and seeing the different kinds of error messages makes me think that all of these failures is due to OMTC not finish compiling before the parallel test finishes. If OMTC is on, ForkJoin runs the function passed to it sequentially while waiting for OMTC to finish. If it finishes executing before OMTC does, tests that expect successful parallel execution will get a "status=warmup" message, and tests that expect failure/bailout will get a "compilation required" message, which is what we see in those logs. Not sure what the best way to fix this is: we can always wait for OMTC if a "mode" is passed in to ForkJoin, or we can wait for OMTC only if we're running inside a test harness.

Shu -- that is not how it's supposed to work. There is a "compile" phase that is supposed to block until compilation completes, in anticipation of precisely this problem. After compile phase completes, we then reiterate, this time expecting compiled versions to be ready. Maybe that compile phase isn't working right.

(In reply to Niko Matsakis [:nmatsakis] from comment #10) > Shu -- that is not how it's supposed to work. There is a "compile" phase > that is supposed to block until compilation completes, in anticipation of > precisely this problem. After compile phase completes, we then reiterate, > this time expecting compiled versions to be ready. Maybe that compile phase > isn't working right. Pushed a try with spewing forced on and the backout un-backed out: https://tbpl.mozilla.org/?tree=Try&rev=d0e96b7be1ab Maybe it'll tell us something new.

That'll teach me to push to try off of inbound instead of central...

OMTC can race with the PJS compilation model. This patch has PJS wait in non-normal (i.e. testing) modes for all OMTC workers to finish, trigger the operation callbacks, and service the callbacks before continuing.

Stupid bzexport auto-assigning the bug on patch upload

Oops, left a deubg printf in.

Terrence, if 949916 sticks, could you try relanding your patches?

i trust you guys are handling this, clearing my "needinfo?"

And relanding: https://hg.mozilla.org/integration/mozilla-inbound/rev/df3c2a1e86d3