1734087 - OCaml playground very slow in Firefox

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Jan de Mooij [:jandem]]

See the Discourse post here: https://discourse.mozilla.org/t/performance-problem-with-large-javascript-blob/86823

It looks like the JS code is throwing a ton of exceptions and we're spending a lot of time capturing stack traces.

Comment 1

[Jan de Mooij [:jandem]]

If I change ThrowOperation to stop capturing a stack trace, it takes less than 5 seconds, confirming the stack capturing is the problem.

I did some experiments and I think Chrome only captures a stack trace for this if the console is open. The website also has a warning about devtools causing severe slowdowns and in that case Chrome is much slower than Firefox, it takes more than 220 seconds.

We probably want to do something similar to Chrome. We could also capture a stack only for the first N exceptions per realm or something if devtools isn't used.

Comment 2

[Lars T Hansen [:lth]]

Maybe more an issue for wasm than for JS in the long term, but we do expect exceptions to be used for control flow when compiling (relatively) exotic languages.

It's my understanding that V8 also has a hack that allows content to ask stack trace generation to be disabled (though I'm uncertain about the scope of that).

Some discussion: https://github.com/WebAssembly/exception-handling/issues/183.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1734087 part 1 - Don't capture a stack trace for every throw-statement in non-debuggee realms. r?sfink!

Stack walking is slow. Match Chrome by only capturing an exception stack if the
realm is a debuggee. Additionally, we also capture stacks for the first 50 throws
per realm to not penalize debugging of JS code that doesn't throw a ton of exceptions.

Note that this is unrelated to Error.stack. That's handled separately and is observable
from JS. The stacks here are used by the debugger for uncaught exceptions that are
not Error objects.

This splits setPendingExceptionAndCaptureStack in versions that always-capture or
maybe-capture.

Comment 4

[Jan de Mooij [:jandem]]

(In reply to Lars T Hansen [:lth] from comment #2)

Maybe more an issue for wasm than for JS in the long term, but we do expect exceptions to be used for control flow when compiling (relatively) exotic languages.

Yeah, we probably don't want to be capturing stack traces for each of these exceptions. We currently cap the number of frames to 128 but that's still a lot of frames (the website here has more frames than that in most cases, hence the excessive slowdown). For Wasm exceptions it's good V8 is at least as slow when it comes to collecting stack traces.

(This is all unrelated to Error.stack, that one is observable so not much we can do there.)

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1734087 part 2 - Add getExceptionInfo testing function and tests. r?sfink!

This allows writing tests for the changes in the previous patch.

Depends on D127531

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/01b38a635903
part 1 - Don't capture a stack trace for every throw-statement in non-debuggee realms. r=sfink
https://hg.mozilla.org/integration/autoland/rev/6d23dc72fe8a
part 2 - Add getExceptionInfo testing function and tests. r=sfink

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/01b38a635903
https://hg.mozilla.org/mozilla-central/rev/6d23dc72fe8a

Comment 8

[Worcester12345]

Needs perf key word.