1895957 - Use emitCalleeGuard in GetPropIRGenerator::tryAttachScriptedProxy

Status: RESOLVED FIXED

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

Description

[Iain Ireland [:iain]]

Charts-chartsjs creates proxies using an object literal containing function literals as the handler. Every proxy created using these handlers will have a unique handler object and a separate instance of each handler function. The scripted proxy get optimizations that Alex landed in bug 1824051 expect a constant handler function, so this code goes generic and ends up calling the fallback repeatedly. In similar cases involving regular calls, we fall back to guarding on the script when attaching non-first stubs: even when there's a fresh function instance each time, they all frequently share the same script. We can probably use the same approach here.

Comment 1

[Iain Ireland [:iain]]

Attachment: Bug 1895957: Use emitCalleeGuard for scripted proxy get r=jandem

Chartjs creates proxies using an object literal containing function literals as the handler. This breaks the scripted proxy get fastpath, which uses GuardSpecificFunction. We can fix this with the same trick we use for handling lambdas elsewhere by guarding on the script if necessary.

Before we do that, though, we have to rewrite the code so that we always call the trap function we load from the handler, instead of guarding that it's what we expect and then hardcoding the callee.

This patch is about 4x faster on a microbenchmark. It looks like a ~2% improvement on chartjs.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2918cc467955
Use emitCalleeGuard for scripted proxy get r=jandem

Comment 3

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/2918cc467955

Comment 4

[Alex Finder| PTO returning Dec. 8th]

(In reply to Pulsebot from comment #2)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2918cc467955
Use emitCalleeGuard for scripted proxy get r=jandem

Perfherder has detected a browsertime performance change from push 2918cc46795569ad8c1fea0d3a67e495c0b6e2dd.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
5%	speedometer3 Charts-chartjs/Draw opaque scatter/Sync	windows10-64-nightlyasrelease-qr	fission webrender	28.14 -> 26.71	Before/After
5%	speedometer3 Charts-chartjs/Draw opaque scatter/total	windows10-64-nightlyasrelease-qr	fission webrender	28.91 -> 27.48	Before/After
3%	speedometer3 Charts-chartjs/Draw opaque scatter/total	macosx1015-64-nightlyasrelease-qr	fission webrender	20.75 -> 20.12	Before/After
3%	speedometer3 Charts-chartjs/Draw opaque scatter/Sync	macosx1015-64-nightlyasrelease-qr	fission webrender	20.15 -> 19.54	Before/After
3%	speedometer3 Charts-chartjs/total	windows10-64-nightlyasrelease-qr	fission webrender	96.17 -> 93.27	Before/After
3%	speedometer3 Charts-chartjs/total	macosx1015-64-nightlyasrelease-qr	fission webrender	76.85 -> 74.60	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 359

For more information on performance sheriffing please see our FAQ.