1804104 - 43% regression on AWFY-Sunspider-access-nsieve on 30-Nov

Status: NEW

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

Description

[Mayank Bansal]

https://treeherder.mozilla.org/perfherder/graphs?highlightAlerts=1&highlightChangelogData=1&highlightCommonAlerts=0&selected=3906886,1608085576&series=mozilla-central,3735747,1,13&series=mozilla-central,3735747,1,13&series=autoland,3906886,1,13&timerange=5184000&zoom=1669572790620,1669907617590,3.658611244005315,8.57031305114662

Comment 1

[Jan de Mooij [:jandem]]

Iain, bug 1797486 is in the range in comment 0. Mind taking a quick look to see if this is reproducible and easily fixable?

Comment 2

[Iain Ireland [:iain]]

It's reproducible. Not immediately sure about easily fixable.

The relevant code looks something like this:

for (var i = 1; i <= 3; i++) {
  var m = (1 << i) * 10000;
  var flags = Array(m+1);
  for (var j = 2; j <= m; j++) {
    flags[j] = true;
  }
  // Do something with flags.
}

We allocate a large array, then we initialize all but the first two elements.

Before my patch, this code had bimodal behaviour:

1. The first few iterations of the inner loop happen in the C++ interpreter. We don't attach any ICs.
2. Eventually, we tier up to blinterp and attach a StoreDenseElementHole IC.
3. We kick off an OSR compilation on a background thread.
   4A: In the fast case, we finish the compilation before the second iteration of the outer loop. Because the pre-patch code supported indices > initializedLength, we don't bail out when writing to flags[2]: the initialized length is 2, but the capacity is 2046. We continue running in Ion with StoreDenseElementHole.
   4B: In the slow case, we start the second iteration of the loop before tiering up to Ion. When we try writing to flags[2], the guard fails, and we instead attach (and eventually transpile) CallAddOrUpdateSparseElement.

After my patch, both baseline ICs and Ion only support writing one past the initialized length, even if we have the capacity.

One option for fixing this would be to add support (in both CacheIR and Ion) for the OOB case where we have the capacity. We'd have to add a loop to fill the gaps with the magic hole value.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

This bug has been marked as a regression. Setting status flag for Nightly to affected.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1797486