2030623 - Delay the disk cache handling to avoid overlapping with performance-sensitive calculation after the pageload

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Tooru Fujisawa [:arai]]

Derived from bug 2029129 comment #12.

In the speedometer3 testcase, the disk cache handling happens during the testcase is still actively running.
Currently the disk cache handling is scheduled with "idle" priority, but apparently that's not enough to avoid overlapping with performance-sensitive calculation after the pageload.
Given that with the stencil navigation cache, the disk cache handling can be done at any time, completely separated than single page load,
we could for example schedule that to perform only when there's no heavy activity for certain amount of period.

For example, schedule the disk cache handling only when there's no pageload activity for 10 seconds:

• In SharedScriptCache::MaybeScheduleUpdateDiskCache, schedule a timer with 10 seconds timeout
  • If there's any existing timer, cancel it, and re-schedule 10 seconds timeout, so that if another pageload happens, that delays the schedule
• only after the 10 seconds timer fully times out, perform the disk cache handling

Comment 1

[Tooru Fujisawa [:arai]]

https://perf.compare/subtests-compare-results?baseRev=ddfef3baa6948e6be0b180b8c27a95a083b0a15c&baseRepo=try&newRev=0ac248c3a62754215ad59d22c8dfb6c922fa6f27&newRepo=try&framework=13&baseParentSignature=5351289&newParentSignature=5351289&test_version=mann-whitney-u&search=svelt+sync+add

The patch fixes the regression.
This was wrong. I compared with wrong "base"

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 2030623 - Delay the disk cache save to avoid overlapping with performance-sensitive timing. r?nbp!

Comment 3

[Tooru Fujisawa [:arai]]

There's slight change, but this doesn't address the regression:

unpatched+disabled vs unpatched+enabled: https://perf.compare/subtests-compare-results?baseRev=0dce7ae2abd37f2b711c3e584525d3d4644a512e&baseRepo=try&newRev=0ac248c3a62754215ad59d22c8dfb6c922fa6f27&newRepo=try&framework=13&baseParentSignature=5351289&newParentSignature=5351289&test_version=mann-whitney-u&search=svelte+add

unpatched+disabled vs patched+enabled vs enabled: https://perf.compare/subtests-compare-results?baseRev=0dce7ae2abd37f2b711c3e584525d3d4644a512e&baseRepo=try&newRev=ddfef3baa6948e6be0b180b8c27a95a083b0a15c&newRepo=try&framework=13&baseParentSignature=5351289&newParentSignature=5351289&test_version=mann-whitney-u&search=svelte+add

So, apparently the regression I observed locally is different than the regression on automation,
or perhaps the test is very noisy and just running 8 iteration locally isn't sufficient.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/887029b44ecf
https://hg.mozilla.org/integration/autoland/rev/d97e3914a8e7
Delay the disk cache save to avoid overlapping with performance-sensitive timing. r=nbp

Comment 5

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/d97e3914a8e7