Closed Bug 1734087 Opened 2 months ago Closed 2 months ago

OCaml playground very slow in Firefox

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox95 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug, )

Details

(Keywords: perf)

Attachments

(2 files)

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.

Flags: needinfo?(jdemooij)

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.

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.

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.

Assignee: nobody → jdemooij
Status: NEW → ASSIGNED

(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.)

Flags: needinfo?(jdemooij)
Attachment #9244321 - Attachment description: Bug 1734087 - Don't capture a stack trace for every throw-statement in non-debuggee realms. r?sfink! → Bug 1734087 part 1 - Don't capture a stack trace for every throw-statement in non-debuggee realms. r?sfink!

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

Depends on D127531

Pushed by jdemooij@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch

Needs perf key word.

You need to log in before you can comment on or make changes to this bug.