Open Bug 1618391 Opened 5 years ago Updated 2 years ago

Avoid parsing and compiling self-hosted code during startup (takes 90ms on Android Moto G5)

Categories

(Core :: JavaScript Engine, defect, P2)

All
Android
defect

Tracking

()

Performance Impact medium

People

(Reporter: mstange, Unassigned)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

(Keywords: perf:responsiveness)

Profile: https://perfht.ml/2wLuQGE

In this profile of App Link startup on Android (on a Moto G5), we spend 95ms inside JS::InitSelfHostedCode doing "script parsing" and "script emit". I was under the assumption that the startup cache should help avoid these costs during startup. Is it not being used for self-hosted code?

The script pre-loader only handle JSM, not the self-hosted code which is baked in the binary of the JS shell.

Ideally we should pre-compile Stencil format ahead of time, or have a way to store it like we do with the Script pre-loader.

Pre-compiling is most appealing solution from my point of view, but it would imply that we are capable of building the same source twice, especially when cross-compiling, in order to generate the Stencil scripts from the frontend.

Priority: -- → P2
Summary: Self-hosted code is not cached in the startup cache? 90ms in JS::InitSelfHostedCode during startup on Android doing script parsing and bytecode emission → Avoid parsing and compiling self-hosted code during startup (takes 90ms on Android Moto G5)

I agree doing this at compile time sounds preferable, but it also sounds harder. If we can get an imperfect solution that still has this overhead on first run but not on subsequent runs, and if it's easier / faster to implement that solution, then I think we should go for it.

(possibly even p1)

Whiteboard: [qf] → [qf:p2:responsiveness]

This is also Bug 1458339 for the Fission memshrink point of view.

The biggest blocker before was the cross-compile issue that :nbp mentions above. Another annoyance here is that if it is not a build-step, the jsshell (which powers a lot of our tests) will need to manage a stateful cache file too and we will have multiple code-paths for critical initialization code.

Self-hosted JS is slightly exotic and there are few things that are not directly supported in the bytecode-cache format. This will all be much better once Bug 1586771 is complete, but even the intermediate work probably simplifies the work here.

In the long run with Bug 1586771 complete and the new frontend in Rust, we will be able to have a clean and robust solution here. A fix in the meantime will certainly be haphazard, but quite possibly justified for Fenix and Fission.

Has this become actionable these days?

Flags: needinfo?(tcampbell)

This issue is on our active TODO list now. Bug 1662149 adds support for using bytecode caching for self-hosted init. Then we'll need to plumb it into the android world. I expect 85 is when this will ship.

Depends on: 1662149
Flags: needinfo?(tcampbell)

Awesome!

During start-up should be clarified as Parent process startup and child process startup:

  • For the parent process using XDR implies reading an extra file, as opposed to using the source baked in the binary. (no bug yet)
  • For the child process using XDR can be done by relying on the data provided by the parent process through shared memory. (Bug 1458339)

(In reply to Markus Stange [:mstange] from comment #2)

I agree doing this at compile time sounds preferable, but it also sounds harder. If we can get an imperfect solution that still has this overhead on first run but not on subsequent runs, and if it's easier / faster to implement that solution, then I think we should go for it.

So far, data collected on our CI suggests that the parent process optimization could save ~16ms from the startup on Android (Android 8.0 Pixel2 AArch64). Which suggests that we might be able to do better on the child processes, as using shared memory should spare us the I/O cost.

Depends on: 1458339

This work is blocked in Bug 1710884, which requires that unrelated test timeout are fixed prior landing.

Depends on: 1710884
Performance Impact: --- → P2
Whiteboard: [qf:p2:responsiveness]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.