1623505 - 20ms in JSFunction::delazifyLazilyInterpretedFunction during content process startup on Moto G5

Status: NEW

(Core :: JavaScript Engine, defect, P3)

Description

[Markus Stange [:mstange]]

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

I'm seeing delazifyLazilyInterpretedFunction show up during content process startup on Android, for JS code in JSM files such as GeckoViewUtils.jsm. It seems to be doing JS parsing and bytecode emission.
What does this mean? Can we avoid it?

These scripts should be coming from the startup cache. Can we parse and emit all functions ahead of time for those?

This code is running on the critical path for "App link" navigations on Android.

Comment 1

[Markus Stange [:mstange]]

Hmm, it's possible that GeckoViewUtils.jsm is not in the startup cache after all.

Comment 2

[Nicolas B. Pierron [:nbp]]

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

I'm seeing delazifyLazilyInterpretedFunction show up during content process startup on Android, for JS code in JSM files such as GeckoViewUtils.jsm. It seems to be doing JS parsing and bytecode emission.
What does this mean? Can we avoid it?

That means that we are parsing script to generate the bytecode.

IIRC, the startup cache of Firefox, used for JSM, does not learn from the execution of it, as opposed to the web-page cache, which waits a bit before recording the bytecode of function which got executed.

I think this can be fixed, but it might be preferable to wait until Stencil is ready to improve our JavaScript caches. This limitation comes from the fact that our bytecode expect to be able to mutate some object, and storing these objects after the execution is incorrect and can lead to wrong execution. (if it does not assert before) However, I would not be surprised if this limitation got removed in October, by Chris Fallin, as he worked on the JSOP_OBJECT opcode.

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

These scripts should be coming from the startup cache. Can we parse and emit all functions ahead of time for those?

Yes, we could do that, as it used to be done in the previous start-up cache. However, doing so generate a HUGE amount of data, which might be even slower to load from the disk compared to loading the sources and compiling it. (based on some old performance team saying)

CC-ing:

• Kmag who implemented the start-up cache of Firefox.
• Ted who is working on Stencil.

Comment 3

[Kris Maglione [:kmag]]

So, in general, all scripts stored in the startup cache are parsed eagerly, and should always have full bytecode. It's possible that we do something different on GeckoView, though.

That said, if we're talking about the ScriptPreloader cache, it should learn from the execution of the script, at least in the session where it's first parsed, and if the script is loaded into the shared JSM global. When a script is loaded during startup, we store a reference to it, and then 10 seconds after startup is complete, we encode any scripts that weren't initially loaded from the session's cache. If those scripts didn't need to be cloned to another compartment in order to run, then we'll take into account the current lazification/delazification state of any of its functions.

Comment 4

[Markus Stange [:mstange]]

Thanks! In the meantime, in bug 1623518 I discovered that the parent process writes out the cache before the content process has had the chance to send its ScriptPreloader data up, so it seems that content process scripts never end up in the cache. We should revisit this bug after that one is fixed.

Comment 5

[Nicolas B. Pierron [:nbp]]

One note about incremental-encoding. When the incremental-encoding mode is enabled there is some overhead on the execution of JavaScript code. This slow down were between 2% - 10% of page-load metrics 3 years ago.

Thus, if the content process is always attempting to record scripts which are always using incremental-encoding and failing to do so, this would cause some slow-down of the JavaScript execution, even after the failed attempt to save the encoded bytecode.

Comment 6

[Nicolas B. Pierron [:nbp]]

Setting priority to P3, as we are not going to work on it from the JS team point of view, despite working towards it in the background.
However, I suggest to move this bug to another component where the ScriptPreloader belongs, if this performance issue cannot wait for Stencil.
Of course, after the investigation why the content process never end up in the cache (comment 4).

Comment 7

[Firefox Bug Husbandry Bot]

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)