1839078 - IonCompileScriptForBaselineOSR called 658910 times for generator function in PerfDashboard benchmark

Status: RESOLVED FIXED

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

Description

[Jeff Muizelaar [:jrmuizel]]

We spend 10x as much time as Chrome in the following function:

*[Symbol.iterator]() {
    const data = this._data;
    const afterEnd = this._afterEndingIndex;
    let i = this._startingIndex;
    for (let i = this._startingIndex; i < afterEnd; ++i)
        yield data[i];
}

The bulk of that time is in js::jit::IonCompileScriptForBaselineOSR which we call 658910 times. This seems suspiciously high to me.

Firefox: https://share.firefox.dev/42LDCh5
Chrome: https://share.firefox.dev/3CDjEux

If we can get to Chrome speed we'll improve our score on this benchmark by 6.4%

Comment 1

[Markus Stange [:mstange]]

Attachment: shell testcase

SM: 32ms https://share.firefox.dev/3Xc30f0
V8: 10ms https://share.firefox.dev/3CBf7J6
JSC: 11ms https://share.firefox.dev/3Jmtncl

Comment 2

[Jan de Mooij [:jandem]]

IonCompileScriptForBaselineOSR is successful, so we enter Ion code via OSR but we then yield immediately to the caller.

We can only resume generator code in Baseline, so we're stuck in a cycle of resume generator in Baseline => OSR to Ion code => yield immediately. We spend so little time in Ion code that the overhead from OSR is much worse than just running it in Baseline.

Comment 3

[Jan de Mooij [:jandem]]

Short-term we probably need some heuristic to not enter Ion for generators via OSR in this case.

Longer-term we should redo our implementation to support resuming in Ion code. Other engines transform generators to look more like a normal function with a switch-statement to make JIT support easier.

Comment 4

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

Local testing of the simplest "Hey, what if we don't warp compile yield in a loop" patch speeds Markus' shell testcase up by 50%.

Comment 5

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

Attachment: Bug 1839078 - Add a loop-analysis heuristic for Warp compiled generators r?jandem

The general thrust of this heuristic is that we allow yield to be Ion compiled under two
circumstances:

1. There is is sufficient bytecode that we believe it may be profitable to Ion compile
2. There is an inner loop without a yield that suggests there will be enough work to avoid
   OSR overhead.

This patch tested on SP3 improved the PerfDashboard subtests by ~4%.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c45e12588c5b
Add a loop-analysis heuristic for Warp compiled generators r=jandem

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/c45e12588c5b