1741787 - Browsertime jobs on CI doesn't cover JavaScript bytecode cache

Status: RESOLVED FIXED

(Testing :: Raptor, task, P2)

Description

[Tooru Fujisawa [:arai]]

Currently browsertime jobs on CI uses --chimera mode, that runs 25 browser cycles and 1 load for cold, 1 load for warm.
This is different than previous configuration (maybe before browsertime) where "cold" used 25 browser cycles and 1 page cycle, and "warm" used 1 browser cycle and 25 page cycles.

JavaScript bytecode cache works when a certain resource is fetched more than 4 times
(a bytecode cache for the JS file is created at 4th fetch, and 5th+ fetches use the cache, instead of compiling the JS file).

It was covered by the previous configuration's "warm" (20/25 page loads used cache),
but it's no longer covered by browsertime --chimera mode, and the bytecode cache performance isn't represented in the score, and it means we're not much tracking the performance impact there.

can we do either of the following?

• change the "warm" to use 5th page load in chimera mode
• add yet another item than "cold" and "warm" that uses 5th page load

Comment 1

[Greg Mierzwinski [:sparky]]

:arai, out of curiosity, is there a way we could force bytecode caching at the first fetch? If so, we could introduce that change so that the first warm includes the cache performance.

If not, then I think changing the warm pageload to the 5th cycle would be fine. As long as we consistently target a single cycle, we'll still have good results (low noise).

Comment 2

[Nicolas B. Pierron [:nbp — off until 29-09]]

(In reply to Greg Mierzwinski [:sparky] from comment #1)

:arai, out of curiosity, is there a way we could force bytecode caching at the first fetch? If so, we could introduce that change so that the first warm includes the cache performance.

You could use dom.script_loader.bytecode_cache.strategy to use an eager encoding strategy.
But for saving, the only thing to do it to let the page go idle after that, that that the idle handler can schedule the encoding of all decoded functions.

The bytecode cache captures functions executed between the navigation start and the idle event. Thus there is no mean to pre-fill the bytecode cache as it is dependent on the execution. Also fast reloads, would prevent the cache from being saved as there is no idle time to do so.

A solution which is implemented in the test suite, is to set dom.expose_test_interfaces = true and to watch for "scriptloader_bytecode_saved" events.

Comment 3

[Greg Mierzwinski [:sparky]]

:nbp, thanks for the information! This is great to know.

So one thing I am wondering is how long would we need to idle to trigger the saves? (Or how long does it take to start seeing events in the event listener).

Comment 4

[Nicolas B. Pierron [:nbp — off until 29-09]]

(In reply to Greg Mierzwinski [:sparky] from comment #3)

So one thing I am wondering is how long would we need to idle to trigger the saves? (Or how long does it take to start seeing events in the event listener).

I do not know, usually the idle event is triggered slightly after the page load.

Knowing that "scriptloader_bytecode_saved" are using the event loop to bubble up this to JavaScript event listeners, and that all scripts in a single document are batched together for saving the bytecode. I would expect that when the idle event loop is processed, it will schedules these new events in the non-idle event loop.

Thus, I presume that you will see batches of "scriptloader_bytecode_saved" event for each document in the page, followed by the processing of the idle-events for other documents within the page (such as iframes).

The easiest way might be to wait for the page to be loaded, to then dynamically insert an iframe in the page which load a script file which will trigger the "scriptloader_bytecode_save" event in the document of the iframe. This event can then be used to trigger the page reload. This is only needed for saving the bytecode to the cache, loading from the cache poses no issues in terms of reloads.

Waiting for the page to be loaded before inserting the iframe, is a way to insert the script event last in the idle-event list. If the page is doing similar things, either we ignore these, or we should find another way to save after. Ignoring them might be ok.

Comment 5

[Greg Mierzwinski [:sparky]]

Thanks again for the information :nbp and thank you for reporting this :arai.

We'll discuss this issue further at our Bi-weekly Performance Strategy Meeting. We have quite a few options for what we could do here and each have their own pros/cons. Here's the list of options we have now:

1. Add a pref and an event listener to force the bytecache to save on the first load (during cold).
   • Not a realistic scenario. Cold load results would be affected and change in “unexpected” ways given that this is not a default behaviour.
2. Do 5 pageloads instead of 2 and create “warm-cached” to represent the 5th.
   • Tests with this change: cold, warm, warm-cached
3. Do 4 pageloads instead of 2 and create “warm-cached” to represent the 4th with a cache and “warm-overhead” to represent pre-cache run when it is being populated.
   * We still have the normal “warm” in this case. Used in conjunction with the pref+event listener.
   * Tests with this change: cold, warm, warm-overhead, warm-cached
4. Do 3 pageloads instead of 2 and create “warm-cached” to represent the 3rd and . Used in conjunction with the pref+event listener.
   * We no longer have a normal “warm” in this case. Used in conjunction with the pref+event listener.
   * Tests with this change: cold, warm-overhead, warm-cached

Comment 6

[Greg Mierzwinski [:sparky]]

A small update for everyone.

After discussing this at the strategy meeting, we decided that (1) was the best solution to use. Here's a try run comparing the results with and without the prefs (and a 20 second idle to trigger the save): https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=0839de92abeb50f763c9a41aa25126c7ac6cf2ff&newProject=try&newRevision=64170db52244ffb10eaec15e9f64a1bd46869d0f&framework=13&page=1&showOnlyImportant=1

:denispal was looking into the results there and was also looking to add telemetry to see how often the bytecode cache gets used. :denispal, I've ni?'ed you so you can provide an update when you're ready.

Comment 7

[Nicolas B. Pierron [:nbp — off until 29-09]]

One issue might be that the eager strategy discard the source length as heuristic:
https://searchfox.org/mozilla-central/source/dom/script/ScriptLoader.cpp#3009

Another problem, that Denis might be able to test easily, might be that we recently switched from parsing lazily to parsing eagerly. Parsing eagerly will fill the bytecode cache with a lot more functions than when it got deployed. The bytecode cache was designed to wait for the idle time to encode every function used, and not encode any function which are unused, thus limiting the disk-space associated with the bytecode cache. Having a larger disk space associated will slow down the warm cases compared to not loading the bytecode. (but not by 700%)

Comment 8

[Denis Palmeiro [:denispal]]

The regressions :sparky saw in first visual change when the bytecode cache is used are caused by some poor scheduling choices in the browser. Since we don't need to wait for the blocking scripts to compile off thread when the cache is available, we end up running scripts immediately, and for quite a while too. This blocks the main thread from painting. In the gslides example, this prevents the svg of the first slide from being painted which is a good 70% of the viewport.

This has been a known problem for a while now, and is also exacerbated without network delays. I don't think it should block us from enabling the bytecode cache for the warm loads, however.

Comment 9

[Eng_Esther]

Attachment: WIP: Bug 1741787 in progress

Comment 10

[Eng_Esther]

Attachment: WIP: Bug 1741787 - Enable bytecode caching for warm pageload tests

Depends on D141903

Comment 11

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Can we have some more explained steps here for Esther? I recommended setting the bytecode cache pref to enabled and pushed a try, but I'm not really into the event listener thing (don't get its use here).

Comment 12

[Nicolas B. Pierron [:nbp — off until 29-09]]

(In reply to Alexandru Ionescu (needinfo me) [:alexandrui] from comment #11)

Can we have some more explained steps here for Esther? I recommended setting the bytecode cache pref to enabled and pushed a try, but I'm not really into the event listener thing (don't get its use here).

The problem we are trying to solve is to make the bytecode cache load cached content while benchmarking. This implies that we are able to save the content to the cache. However, saving the content to the cache implies that the task in charge of saving it is scheduled.

Task in charged of saving the bytecode is currently scheduled in the idle event handler, which comes some time in the future. As we do not know precisely when it comes, the patches rely on some testing-only events added to validate the feature. As the bytecode caching has no user visible effect, except on timing, events were added to make this feature visible when the preference dom.expose_test_interfaces is set. Capturing these events is the most effective way to guarantee that the content of the bytecode cache is saved.

Not watching for these testing events implies that we use other ways to wait for the fact that the idle event queue becomes empty.

Comment 13

[Eng_Esther]

Attachment: WIP: Bug 1741787 - bytecode caching for warm pageload tests -different approach

Depends on D142266

Comment 14

[Eng_Esther]

Attachment: WIP: Bug 1741787 : Test bytecode cache pref changes

Comment 15

[Eng_Esther]

Attachment: WIP: Bug 1741787 - add delay for bytecode caching

Comment 16

[Eng_Esther]

Attachment: Bug 1741787: add 20 sec delay after commands.measure.start

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/643fad8de973
add 20 sec delay after commands.measure.start r=perftest-reviewers,sparky

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6977506c8456
Fix lint failure in testing/raptor/raptor/browsertime/base.py CLOSED TREE

Comment 19

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/643fad8de973
https://hg.mozilla.org/mozilla-central/rev/6977506c8456

Comment 20

[Nicolas B. Pierron [:nbp — off until 29-09]]

What is the reasoning for flagging this bug as mozilla-employee-confidential ?