1858679 - Callee guard for self-hosted lambda functions can cause unnecessary guard failures and bailouts/recompiles

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task)

Description

[Jan de Mooij [:jandem]]

In IRGenerator::emitCalleeGuard, we currently always emit GuardSpecificFunction for self-hosted builtins. This is the right thing in most cases, but not for self-hosted lambda functions such as the one returned by ArraySortCompare, because we'll attach a new Baseline IC stub for each clone and this can cause unnecessary bailouts and recompiles.

Comment 1

[Jan de Mooij [:jandem]]

Below is a simple example where we do 24 Warp compilations (according to IONFLAGS=scripts). Fixing this reduces it to 14.

function f() {
    var a = [];
    for (var i = 0; i < 2000; i++) {
        a.push({x: i});
    }
    for (var i = 0; i < 10; i++) {
        a.sort((a, b) => a.x - b.x);
    }
}
f();

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1858679 part 1 - Fix perf issue in IRGenerator::emitCalleeGuard for self-hosted lambdas. r?iain!

We were always emitting GuardSpecificFunction for self-hosted functions, but this
doesn't work well for self-hosted lambda functions because there we have lambda clones
and we want to guard on the script instead.

Lambdas aren't very common in self-hosted code, but ArraySortCompare returns a lambda
function and that's where it was causing unnecessary bailouts and recompilations.

Comment 3

[Jan de Mooij [:jandem]]

The perf comparison below shows some medium-confidence 3-5% improvements on various Speedometer 3 subtests. I'm not sure how accurate this is, but I do expect this to get rid of some bailouts and compilations.

https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=b55ea1ae7acca29cdfadc2b6d893f4814d597dc3&originalSignature=4586009&newSignature=4586009&framework=13&application=firefox&originalRevision=0555f9e77051726fc84cda22912582175d1b2975&page=1

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1858679 part 2 - Remove dead code from CloneFunctionReuseScript. r?iain!

Searchfox suggested the code to handle self-hosted lazy functions is dead and
this is indeed no longer used to clone lazy self-hosted functions.

Depends on D190821

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3a7286b836d
part 1 - Fix perf issue in IRGenerator::emitCalleeGuard for self-hosted lambdas. r=iain
https://hg.mozilla.org/integration/autoland/rev/810153993fc0
part 2 - Remove dead code from CloneFunctionReuseScript. r=iain

Comment 6

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/d3a7286b836d
https://hg.mozilla.org/mozilla-central/rev/810153993fc0