1824772 - Explore caching jit hints in the process for eager baseline compilation

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P2)

Description

[Denis Palmeiro [:denispal]]

:smaug had an idea to cache warmup counters in the process so that we can skip the warmup iterations if we ever encounter the same function again as a new JSScript. I put together a rough prototype to test things out and results look fairly promising. The prototype is pretty unrealistic as it is, but there does seem to be some potential in doing something like this but in a better way.

unbounded hash table:
https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=4211b09834c6efa482a6513c3f4afa3542174235&originalSignature=4569240&newSignature=4569240&framework=13&application=firefox&originalRevision=45f80789a6a522b740b7ea9e4593cf0db184621f&page=1&showOnlyImportant=1

mrucache:
https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=a3af1874ae7385c23ad951cd4e1d7a41ce801ee6&originalSignature=4586009&newSignature=4586009&framework=13&application=firefox&originalRevision=2b99bf0cd3cfd7e3a02ad111c0f54edd82519f65&page=1&showOnlyImportant=1

Comment 1

[Denis Palmeiro [:denispal]]

I will explore some more sane caching implementations next so that the tables are not so large.

Comment 2

[Nicolas B. Pierron [:nbp]]

Can you clarify what "jit hints" means, and whether this is useful for users or only for benchmarketting purposes?

We long had ideas of adding Jit hints, but only as part of the necko cache, which is not the same as a per-process cache.

Also, if you are looking at a per-process cache, then we should probably revive the intent of caching stencils across page reloads / navigation if there is definitely some interest for users.

Comment 3

[Denis Palmeiro [:denispal]]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)

Can you clarify what "jit hints" means, and whether this is useful for users or only for benchmarketting purposes?

We long had ideas of adding Jit hints, but only as part of the necko cache, which is not the same as a per-process cache.

Also, if you are looking at a per-process cache, then we should probably revive the intent of caching stencils across page reloads / navigation if there is definitely some interest for users.

The idea is to keep an in memory cache to record some basic info about to identify the script being executed (e.g. filename+sourceStart, or filehash+sourceStart) and whether it has entered baseline before and then if we encounter this function again in a navigation, reload or whatever, then we can skip the warmup and just eagerly baseline compile. So far it looks promising on benchmarks, but it does regress pageload by 2-4% so there needs to be tuning done there if we go with this approach.

Comment 4

[Nicolas B. Pierron [:nbp]]

(In reply to Denis Palmeiro [:denispal] from comment #3)

The idea is to keep an in memory cache to record some basic info about to identify the script being executed (e.g. filename+sourceStart, or filehash+sourceStart)

I think it would be better to rely on the cache entry ID provided by necko instead of re-inventing the wheel. The filename does not guarantee the uniqueness, and a hashes can theoretically have collision.

and whether it has entered baseline before and then if we encounter this function again in a navigation, reload or whatever, then we can skip the warmup and just eagerly baseline compile.

Have you looked at populating all baseline function eagerly, instead of gathering hints?

We discussed about doing off-thread, in addition to the concurrent delazification task.

Comment 5

[Denis Palmeiro [:denispal]]

(In reply to Nicolas B. Pierron [:nbp] from comment #4)

Have you looked at populating all baseline function eagerly, instead of gathering hints?

Yes, it gives roughly the same results on benchmarks. I imagine forcing eager baseline is much worse on pageload, but we've discussed possibly enabling it after the page has loaded instead of using hints.

Comment 6

[Denis Palmeiro [:denispal]]

Attachment: Bug 1824772: part 1 - Extend bloom filter to support an additional hash value. r=arai

The current bloom filter uses only 2 hash functions. Adding support for an additional hash value will give us another 2 hash functions to reduce false positives.

Comment 7

[Denis Palmeiro [:denispal]]

Attachment: Bug 1824772: part 1 - Add jit option and static pref to toggle eager baseline hints. r=iain!

Add a pref, javascript.options.jitHints, to toggle eager baseline hints.

Depends on D175520

Comment 8

[Denis Palmeiro [:denispal]]

Attachment: Bug 1824772: part 2 - Hash the filename used by script source and cache it. r=iain!

The hashed filename + sourceStart is used as a key for the in-process eager baseline hint cache. Caching the filename hash in the script source also avoids the need to hash it repeatedly.

Depends on D175521

Comment 9

[Denis Palmeiro [:denispal]]

Attachment: Bug 1824772: part 3 - Add a mutable script flag to indicate if an eager baseline hint is unavailable. r=iain!

This flag is not absolutely necessary, but it does help save some cycles and prevents additional lookups for the same script if we couldn't find it in the cache the first time.

Depends on D175522

Comment 10

[Denis Palmeiro [:denispal]]

Attachment: Bug 1824772: part 4 - Cache jit hints in the process for eager baseline compilation r=iain!,arai!

Add a cache, based on a bloom filter, in the JIT Runtime that will exist for the process lifetime. Whenever a script is baseline compiled, we add a hint to the cache based on the script filename+sourceStart values. If we encounter this script again, during a navigation for example, then we can eagerly compile this script and skip the warmup.

Using a bloom filter does introduce false positives, so we try to maintain a false positivity rate of less than 1% and if we exceed this value we clear the cache.

Depends on D175523

Comment 11

[Iain Ireland [:iain]]

For posterity: this is the calculator we have been using to estimate the false positive rate: https://hur.st/bloomfilter

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3740c123c6b8
part 1 - Add jit option and static pref to toggle eager baseline hints. r=iain
https://hg.mozilla.org/integration/autoland/rev/1e10afa76d16
part 2 - Hash the filename used by script source and cache it. r=iain
https://hg.mozilla.org/integration/autoland/rev/38c9bdbe1d9a
part 3 - Add a mutable script flag to indicate if an eager baseline hint is unavailable. r=iain
https://hg.mozilla.org/integration/autoland/rev/6895dba1dca7
part 4 - Cache jit hints in the process for eager baseline compilation r=iain,arai

Comment 13

[Narcis Beleuzu [:NarcisB]]

Backed out for bc failures on browser_tabMatchesInAwesomebar.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/e3bcc399fbe6ac4ee949f4c5e4ee257c9fa5283f
Log link: https://treeherder.mozilla.org/logviewer?job_id=413093509&repo=autoland&lineNumber=5116

Please also check: https://treeherder.mozilla.org/logviewer?job_id=413094585&repo=autoland&lineNumber=5354

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/3740c123c6b8
https://hg.mozilla.org/mozilla-central/rev/1e10afa76d16
https://hg.mozilla.org/mozilla-central/rev/38c9bdbe1d9a
https://hg.mozilla.org/mozilla-central/rev/6895dba1dca7

Comment 15

[Denis Palmeiro [:denispal]]

(In reply to Narcis Beleuzu [:NarcisB] from comment #13)

Backed out for bc failures on browser_tabMatchesInAwesomebar.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/e3bcc399fbe6ac4ee949f4c5e4ee257c9fa5283f
Log link: https://treeherder.mozilla.org/logviewer?job_id=413093509&repo=autoland&lineNumber=5116

Please also check: https://treeherder.mozilla.org/logviewer?job_id=413094585&repo=autoland&lineNumber=5354

I cannot reproduce this timeout locally, but looking at the test's history it does seem to happen intermittently although mostly on mac (https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1775821&startday=2023-01-01&endday=2023-04-18&tree=all). It appears that baseline compiling more eagerly on the parent process may cause this timeout to occur more frequently for the linux asan builds, although I'm not entirely sure why. There is some more history with the intermittent timeout's here as well https://bugzilla.mozilla.org/show_bug.cgi?id=1166297#c209.

Increasing the timeout delay does make the test pass, but it seems that just limiting the hints cache to the content process is a more suitable workaround which also makes the test pass.

Comment 16

[Alex Finder]

(In reply to Norisz Fay [:noriszfay] from comment #14)

https://hg.mozilla.org/mozilla-central/rev/3740c123c6b8
https://hg.mozilla.org/mozilla-central/rev/1e10afa76d16
https://hg.mozilla.org/mozilla-central/rev/38c9bdbe1d9a
https://hg.mozilla.org/mozilla-central/rev/6895dba1dca7

== Change summary for alert #38148 (as of Thu, 20 Apr 2023 11:08:33 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
7%	speedometer3	windows10-64-shippable-qr	fission webrender	227.98 -> 243.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=38148

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/694ca1efdbbb
part 1 - Add jit option and static pref to toggle eager baseline hints. r=iain
https://hg.mozilla.org/integration/autoland/rev/18adb7b6e661
part 2 - Hash the filename used by script source and cache it. r=iain
https://hg.mozilla.org/integration/autoland/rev/706595d4c5cb
part 3 - Add a mutable script flag to indicate if an eager baseline hint is unavailable. r=iain
https://hg.mozilla.org/integration/autoland/rev/9d119497d6d6
part 4 - Cache jit hints in the process for eager baseline compilation r=iain,arai

Comment 18

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/694ca1efdbbb
https://hg.mozilla.org/mozilla-central/rev/18adb7b6e661
https://hg.mozilla.org/mozilla-central/rev/706595d4c5cb
https://hg.mozilla.org/mozilla-central/rev/9d119497d6d6

Comment 19

[Andrej (:aglavic)]

(In reply to Norisz Fay [:noriszfay] from comment #14)

https://hg.mozilla.org/mozilla-central/rev/3740c123c6b8
https://hg.mozilla.org/mozilla-central/rev/1e10afa76d16
https://hg.mozilla.org/mozilla-central/rev/38c9bdbe1d9a
https://hg.mozilla.org/mozilla-central/rev/6895dba1dca7

== Change summary for alert #38217 (as of Tue, 25 Apr 2023 02:02:29 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
2%	linkedin LastVisualChange	windows10-64-shippable-qr	fission warm webrender	2,066.71 -> 2,019.25	Before/After
2%	linkedin SpeedIndex	windows10-64-shippable-qr	fission warm webrender	765.67 -> 749.33	Before/After

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=38217