1905987 - Use invalidation to avoid checking generation counters when loading globals

Status: REOPENED

(Core :: JavaScript Engine, task, P3)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

Similar to invalidating fuses, we can recycle the machinery for generation counter changes.

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1905987 - Add CompilationDependencyTracker r?iain

This is an infrastructure commit about adding a centralized place for tracking
the dependencies a compilation acquires as it is processed.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1905987 - Use invalidation for generation counter checking in Warp r?jandem

Comment 3

[Phabricator Automation]

Comment on attachment 9410920 [details]
Bug 1905987 - Add CompilationDependencyTracker r?iain

Revision D215565 was moved to bug 1905988. Setting attachment 9410920 [details] to obsolete.

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Interesting -- previous measurement of this showed this to have a regression, but I'm now measuring a nice tidy speedup across a whole bunch of subtests...

Assuming Bug 1905988 sticks, I'll put this up for review again.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21015e92d7f9
Use invalidation for generation counter checking in Warp r=iain

Comment 6

[Mayank Bansal]

This looks to have improved some tests on Jetstream2:
crypto-md5-SP-Average by 3%
delta-blue-Average by 2%-3%
Mandreel tests by 2.1%
richards-Average by 1.2%

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/21015e92d7f9

Comment 8

[Atila Butkovits]

Backed out as requested.

Backout link: https://hg.mozilla.org/integration/autoland/rev/5a0dd1a4a03f6d4d6d9c098f615339e3ace90711

Comment 9

[Matthew Gaudet (he/him) [:mgaudet]]

Backed out due to Bug 1919295.

At this point, the small jetstream benefits don't seem worth the enormous google docs slowdown.

Comment 11

[Mayank Bansal]

Mildly interesting observation: When this patch landed, there was a tiniest increase of 512bytes on Base Content JS Opt Fission. When this patch got backed out, the numbers returned back.

Comment 12

[Andra Esanu (needinfo me)]

(In reply to Atila Butkovits from comment #8)

Backed out as requested.

Backout link: https://hg.mozilla.org/integration/autoland/rev/5a0dd1a4a03f6d4d6d9c098f615339e3ace90711

Perfherder has detected a browsertime performance change from push fb4318779187f4b4724ae2f56e91608e550520e5.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
51%	google-docs PerceptualSpeedIndex	windows11-64-shippable-qr	fission warm webrender	2,472.82 -> 1,223.05	Before/After
48%	google-docs LastVisualChange	windows11-64-shippable-qr	fission warm webrender	5,142.51 -> 2,653.67	Before/After
45%	google-docs LastVisualChange	linux1804-64-shippable-qr	fission warm webrender	9,622.71 -> 5,263.16	Before/After
42%	google-docs PerceptualSpeedIndex	linux1804-64-shippable-qr	fission warm webrender	4,209.62 -> 2,428.41	Before/After
39%	google-docs ContentfulSpeedIndex	windows11-64-shippable-qr	fission warm webrender	1,308.73 -> 799.97	Before/After
...	...	...	...	...	...
5%	google-docs largestContentfulPaint	macosx1015-64-shippable-qr	fission warm webrender	1,019.25 -> 966.04	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 2191

For more information on performance sheriffing please see our FAQ.