1847258 - Use warmup counter of last attached IC stub as the Ion hint threshold

Status: RESOLVED FIXED

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

Description

[Denis Palmeiro [:denispal]]

In bug 1837192, we used a value of 550 for the Ion hint threshold so that there was minimal impact on inlining decisions. However, it seems like SP3 could benefit from roughly another 1% improvement if we lower the threshold much more aggressively (I used 30 in this run) when there are no inlining candidates.

Comment 1

[Worcester12345]

"Perf" key word?

Comment 2

[Denis Palmeiro [:denispal]]

Attachment: Bug 1847258: Save the warm up counter when the last IC stub is attached. r=iain!

Comment 3

[Denis Palmeiro [:denispal]]

Attachment: Bug 1847258: Use the warmup counter when the last IC is attached as the Ion hint threshold, and adjust inlining heuristics when the hint is used. r=iain!

Depends on D192878

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f9bc02e30c3
Save the warm up counter when the last IC stub is attached. r=iain
https://hg.mozilla.org/integration/autoland/rev/f24223382358
Use the warmup counter when the last IC is attached as the Ion hint threshold, and adjust inlining heuristics when the hint is used. r=iain

Comment 5

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/0f9bc02e30c3
https://hg.mozilla.org/mozilla-central/rev/f24223382358

Comment 6

[Mayank Bansal]

improvement:
2.5% on AWFY-Speedometer2.1 : https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&selected=3912818,1797586757&series=mozilla-central,3735773,1,13&series=autoland,3912818,1,13&timerange=5184000&zoom=1700285367680,1700631683127,163.8637131524225,216.20553663292648

Comment 7

[Cosmin Sabou [:CosminS]]

Backed out from central: https://hg.mozilla.org/mozilla-central/rev/27366586a33a4d0f53cd466bbca28d1edfacf09c

Comment 8

[Denis Palmeiro [:denispal]]

Still looking, but I think it may be possible for icScript->inliningRoot()->owningScript() not to have a jitScript, so we should probably guard against that case.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d387ad455afc
Save the warm up counter when the last IC stub is attached. r=iain
https://hg.mozilla.org/integration/autoland/rev/7dc6122ebd68
Use the warmup counter when the last IC is attached as the Ion hint threshold, and adjust inlining heuristics when the hint is used. r=iain

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d387ad455afc
https://hg.mozilla.org/mozilla-central/rev/7dc6122ebd68

Comment 11

[Mayank Bansal]

The first time these patches had landed, there were some large improvements on some AWFY tests. However, when the patches were backed out (and then subsequently relanded), those perf improvements did not appear.

Example1 :AWFY-Jetstream2-3d-cube-SP-Average
Improvement(74%)
Backout which reverted the improvement too
Relanded with no improvement

Example2: AWFY-Jetstream2-ai-astar-Average
Improvement(15%)
Backout which reverted the improvement too
Relanded with no improvement

Not 100% sure if the improvements were from this bug or some other bug (e.g. bug 1863620 or infra issue etc.)

Comment 12

[Denis Palmeiro [:denispal]]

(In reply to Mayank Bansal from comment #11)

The first time these patches had landed, there were some large improvements on some AWFY tests. However, when the patches were backed out (and then subsequently relanded), those perf improvements did not appear.

Example1 :AWFY-Jetstream2-3d-cube-SP-Average
Improvement(74%)
Backout which reverted the improvement too
Relanded with no improvement

Example2: AWFY-Jetstream2-ai-astar-Average
Improvement(15%)
Backout which reverted the improvement too
Relanded with no improvement

Not 100% sure if the improvements were from this bug or some other bug (e.g. bug 1863620 or infra issue etc.)

Thanks for looking at this, I will try to find some time to see if these improvements can be recovered.

Comment 13

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #9)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d387ad455afc
Save the warm up counter when the last IC stub is attached. r=iain
https://hg.mozilla.org/integration/autoland/rev/7dc6122ebd68
Use the warmup counter when the last IC is attached as the Ion hint
threshold, and adjust inlining heuristics when the hint is used. r=iain

== Change summary for alert #40363 (as of Thu, 23 Nov 2023 05:15:03 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
4%	nytimes SpeedIndex	macosx1015-64-shippable-qr	fission warm webrender	822.58 -> 791.02	Before/After
3%	nytimes loadtime	macosx1015-64-shippable-qr	fission warm webrender	730.50 -> 709.97	Before/After

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

Comment 14

[Andra Esanu (needinfo me)]

(In reply to Pulsebot from comment #9)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d387ad455afc
Save the warm up counter when the last IC stub is attached. r=iain
https://hg.mozilla.org/integration/autoland/rev/7dc6122ebd68
Use the warmup counter when the last IC is attached as the Ion hint
threshold, and adjust inlining heuristics when the hint is used. r=iain

== Change summary for alert #40373 (as of Thu, 23 Nov 2023 12:37:32 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
3%	nytimes LastVisualChange	macosx1015-64-shippable-qr	fission warm webrender	1,032.56 -> 997.13	Before/After

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

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

== Change summary for alert #40354 (as of Thu, 23 Nov 2023 00:43:28 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
6%	damp console.log-in-loop-content-process-longstring	windows10-64-shippable-qr	e10s fission stylo webrender	49.15 -> 51.96
6%	damp console.log-in-loop-content-process-longstring	windows10-64-shippable-qr	e10s fission stylo webrender-sw	49.04 -> 51.79
4%	damp console.log-in-loop-content-process-object	windows10-64-shippable-qr	e10s fission stylo webrender	205.02 -> 212.93
4%	damp console.log-in-loop-content-process-object	windows10-64-shippable-qr	e10s fission stylo webrender-sw	205.02 -> 212.84
3%	damp console.log-in-loop-content-process-document	windows10-64-shippable-qr	e10s fission stylo webrender-sw	76.29 -> 78.29
2%	damp console.log-in-loop-content-process-nodelist	windows10-64-shippable-qr	e10s fission stylo webrender	201.55 -> 206.04

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
18%	damp console.log-in-loop-content-process-node	windows10-64-shippable-qr	e10s fission stylo webrender-sw	46.13 -> 37.66
18%	damp console.log-in-loop-content-process-node	windows10-64-shippable-qr	e10s fission stylo webrender	45.84 -> 37.61

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