Closed Bug 1765779 Opened 1 month ago Closed 1 month ago

Support scalar replacement of array iterator objects

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
101 Branch
Tracking Status
firefox101 --- fixed

People

(Reporter: anba, Assigned: anba)

References

(Blocks 1 open bug)

Details

Attachments

(6 files)

To reproduce:

  • Execute below test case with IONFLAGS=logs
  • Run iongraph --final /tmp/ion.json.
  • Observe that the iterator object wasn't scalar replaced.
function g() {
  var a = [1, 2];
  var r = 0;
  for (var v of a) r += v;
  return r;
}

function f() {
  with ({});
  let r = 0;
  for (let i = 0; i < 100_000; ++i) {
    r += g();
  }
  print(r);
}

with ({});
for (let i = 0; i < 5; ++i) f();

GuardToArrayIterator in ArrayIteratorNext blocks scalar replacement of the
array iterator object because GuardToClass isn't handled.

var obj; is equivalent to var obj = undefined;. This leads to creating a
phi-node, because obj can be either undefined or the this-value. If we
instead initialise var obj = this;, we can ensure that we see only object-typed
definitions during scalar replacement and it ensures we don't emit spurious
MBox instructions for the phi-node input.

Depends on D144290

Pass the object instruction in preparation for the next part.

Depends on D144291

The guard pattern:

var obj = this;
if (!IsObject(obj) || (obj = GuardToArrayIterator(obj)) === null) {
  return callFunction(CallArrayIteratorMethodIfWrapped, this,
                      "ArrayIteratorNext");
}

creates a phi-node for obj. We can replace this phi-node when all its inputs
are equal to the iterator object we try to scalar replace.

Depends on D144292

Scalar replacement for iterator objects requires to support this code:

(obj = GuardToArrayIterator(obj)) === null

Depends on D144293

This is the last remaining instruction which needs to be supported for scalar
replacement of iterator objects.

Depends on D144294

Thank you for these patches!

I worked on the array side of it in the past and this got regressed many times since the removal of AWFY micro-benchmarks that were checking performance differences between C-style loops and for-of loops.

Pushed by andre.bargull@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0b22cda4d2fd
Part 1: Handle GuardToClass in object scalar replacement. r=iain
https://hg.mozilla.org/integration/autoland/rev/6b96d9037d34
Part 2: Change guard-to pattern to avoid spurious MBox instructions. r=iain
https://hg.mozilla.org/integration/autoland/rev/ff95904bc09e
Part 3: Pass object instruction to IsObjectEscaped. r=iain
https://hg.mozilla.org/integration/autoland/rev/c4a0be613d2d
Part 4: Support phi-nodes for iterator object scalar replacement. r=iain
https://hg.mozilla.org/integration/autoland/rev/9be298355200
Part 5: Support MCompare in object scalar replacement. r=iain
https://hg.mozilla.org/integration/autoland/rev/460d0aadbf1e
Part 6: Support MIsObject in object scalar replacement. r=iain
You need to log in before you can comment on or make changes to this bug.