1971446 - Remove hole check from LoadElementV

Status: RESOLVED FIXED

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

Description

[Iain Ireland [:iain]]

We use LLoadElementV to load a value from an array in Ion. When we do, we unconditionally generate a branchTestMagic to ensure that the value loaded is not the magic hole value. When we are looping over a large array, the cost of this check can add up. For example, on this microbenchmark, I get a 25% performance improvement by commenting out the hole check:

function foo(arr, i) {
  return arr[i];
}

function bar() {
  let result = 0;
  for (var i = 0; i < arr.length; i++) {
    result += foo(arr, i);
  }
  return result;
}

with ({}) {}
let arr = [];
for (var i = 0; i < 10000; i++) {
  arr.push(i);
}

let start = performance.now()
for (var i = 0; i < 100000; i++) {
  bar();
}
print(performance.now() - start);

There are more efficient ways to do this. For example, checking the IsNonPacked flag on the elements is probably a bit cheaper and, more importantly, can be hoisted outside the loop because it doesn't depend on the index.

Comment 1

[Debadree Chatterjee]

So we could add a MIR which generates a instruction like https://searchfox.org/mozilla-central/source/js/src/jit/CodeGenerator.cpp#15126-15127 and add it before every place we generate the MLoadElement instruction?

Comment 2

[Iain Ireland [:iain]]

That instruction is setting the NonPacked flag when we store a hole value. We want to guard that the flag is not set, similar to GuardArrayIsPacked, but without the array-specific length == initializedLength check.

I wrote a patch for this yesterday. You can see the try run here.

Comment 3

[Debadree Chatterjee]

Thank you so much for explaining i shall study the patch! :-)

Comment 4

[Iain Ireland [:iain]]

Attachment: Bug 1971446: Use packed elements check instead of hole check where possible for LoadDenseElement r=jandem

Comment 5

[Iain Ireland [:iain]]

Attachment: Bug 1971446: Support GuardElementsArePacked in scalar replacement r=jandem

Comment 6

[Iain Ireland [:iain]]

Attachment: Bug 1971446: Update PBL r=jandem

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cef89dd5f4b1
https://hg.mozilla.org/integration/autoland/rev/d38344c6778c
Use packed elements check instead of hole check where possible for LoadDenseElement r=jandem
https://github.com/mozilla-firefox/firefox/commit/41aeb1ce06b7
https://hg.mozilla.org/integration/autoland/rev/a3d1c99c9cc6
Support GuardElementsArePacked in scalar replacement r=jandem
https://github.com/mozilla-firefox/firefox/commit/25709632f1aa
https://hg.mozilla.org/integration/autoland/rev/83453fc62def
Update PBL r=jandem

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/d38344c6778c
https://hg.mozilla.org/mozilla-central/rev/a3d1c99c9cc6
https://hg.mozilla.org/mozilla-central/rev/83453fc62def