1994205 - Pasting small text on https://opentype.js.org/ is 3x slower in Firefox and spends time in things like IonInstanceOfIC::update, map, and js::BaseShape::clasp

Status: VERIFIED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Mayank Bansal]

Attachment: test.txt

Go to https://opentype.js.org/
Copy-paste the attached sample text
Once the text is pasted, you can try to check/uncheck the options, or play with the sliders of the second demo.

firefox pasting: https://share.firefox.dev/4n5Qg4K (3s)
Firefox all operations: https://share.firefox.dev/43dc4UZ
Chrome: https://share.firefox.dev/43avYQu (1s to paste, all the operations)

Comment 1

[Jan de Mooij [:jandem]]

Interesting find. The code does action instanceof SubstitutionAction where action is a primitive. Our CacheIR implementation of instanceof requires the RHS (SubstitutionAction) to have a .prototype property, but because the LHS is a primitive, we never resolve this lazy property in the fallback code because the spec only does the lookup for LHS objects.

I checked some other benchmarks and apparently the new-ish prismjs test in JetStream 3 also hit this a number of times (for LHS string values).

Comment 2

[Jan de Mooij [:jandem]]

Here's a micro-benchmark that improves from 6863 ms to 284 ms (--spectre-mitigations=off) with a fix for this.

function Foo() {}
function f() {
  var res = 0;
  var t = Date.now();
  for (var i = 0; i < 100_000_000; i++) {
    res += (i instanceof Foo);
  }
  print(Date.now() - t);
  return res;
}
f();

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1994205 part 1 - Tidy up InstanceOfIRGenerator::tryAttachStub a bit. r?iain!

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1994205 part 2 - Resolve .prototype property in InstanceOfIRGenerator if needed. r?iain!

This fixes a performance cliff when the LHS is a primitive. In that case
the fallback code will not resolve the lazy RHS.prototype property but our
CacheIR code currently requires a prototype object for LoadInstanceOfObjectResult.

We could emit different CacheIR ops for this case involving a primitive LHS with a
not-yet-resolved RHS.prototype, but that seemed more complicated.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/48f61ddc9886
https://hg.mozilla.org/integration/autoland/rev/4efd82e08919
part 1 - Tidy up InstanceOfIRGenerator::tryAttachStub a bit. r=iain
https://github.com/mozilla-firefox/firefox/commit/1aadb8502c27
https://hg.mozilla.org/integration/autoland/rev/dd8501220db1
part 2 - Resolve .prototype property in InstanceOfIRGenerator if needed. r=iain

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4efd82e08919
https://hg.mozilla.org/mozilla-central/rev/dd8501220db1

Comment 7

[Mayank Bansal]

Latest Nightly: https://share.firefox.dev/4qkUDf1 (850ms)
So we are 3.5x faster than before and 15% faster than Chrome now.