We have some crashes in PatchJump under InterruptCheck/patchIonBackedges. In bug 1435360 we removed async interrupts for wasm. If wasm can get away with it, it seems it should be okay for Ion too. Furthermore, code patching isn't great (especially with W^X) and it's something I want to avoid as much as possible. In this case it also involves multi-threading and signal handlers; it makes it pretty difficult to reason about this.

This will regress micro-benchmarks in particular. Kraken and Octane-crypto will also regress a bit. I don't like this, but the code for this is just too complicated and we're seeing weird crashes when patching backedges. 49 files changed, 60 insertions(+), 1177 deletions(-)

Removes a bit more code.

Comment on attachment 8963550 [details] [diff] [review] Patch Ugh there's much more we can remove.

61 files changed, 79 insertions(+), 1315 deletions(-)

Comment on attachment 8963560 [details] [diff] [review] Patch Review of attachment 8963560 [details] [diff] [review]: ----------------------------------------------------------------- Wow, that was a lot!

Another problem with urgent interrupts is that they're not just for the (uncommon) slow script dialog case - we call JS_RequestInterruptCallback quite often for workers. There are some pathological cases with console.log called from a worker (that will use a ConsoleRunnable which apparently does an urgent interrupt every time), but I added some logging and worker interrupts also show up on Google Maps etc.

Oh and I think for workers it's often the main thread that does the JS_RequestInterruptCallback and has to do the backedge patching (at least on Windows). Blocking the main thread like this is not great.

Here's a (stupid) worker test case that improves from ~6800 ms ~5300 ms for me on OS X. That suggests we currently spend >20% of the time in pthread_kill et al.

Pushed by jandemooij@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/81ef11104ebb Remove async Ion loop interrupts. r=luke

OK I just pushed this. Let's see how it affects AWFY and Talos - it will likely regress Kraken and other (micro-ish) benchmarks, but overall it should be fine hopefully. A few thoughts: (1) Safari/JSC doesn't have loop interrupt checks (|while(true) {}| just iloops). Luke and I were talking about this and maybe we can do something similar in content processes or use some heuristics (maybe we should wait for project-Fission). (2) It might also be interesting to see if enabling our loop unrolling pass for very short loops helps Kraken.