1709216 - Optimize polymorphic Object.is

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P1)

Description

[Iain Ireland [:iain]]

Our CacheIR for Object.is is currently focused on the case where the types are consistent, similar to any other comparison operator. Relative to most comparisons, however, Object.is is particularly likely to be used in polymorphic code. In particular, React uses Object.is when checking to see if the value of a property has changed (example).

Right now, polymorphic Object.is will end up as a VM call to SameValue. We can speed this up in a number of ways. The easiest is to do a quick bitwise comparison of the two values: because Object.is(NaN,NaN) is true, bitwise-identical values will always return true, so we can skip the VM call.

Performance on microbenchmarks looks pretty good: a 1.67x speedup when the fast path almost always fails (I think because using a dedicated MIR op with AliasSet::None lets us optimize better) and an 8x speedup when the fast path always succeeds.

(Not sure if this moves the needle in real-world code yet.)

Comment 1

[Iain Ireland [:iain]]

Attachment: Bug 1709216: Optimize polymorphic Object.is in CacheIR r=jandem

Comment 2

[Iain Ireland [:iain]]

Attachment: Bug 1709216: Rename SameValue MIR op r=jandem

Rename this to make room for the next patch to use that name when transpiling SameValueResult.

Depends on D114257

Comment 3

[Iain Ireland [:iain]]

Attachment: Bug 1709216: Transpile SameValueResult r=jandem

Depends on D114258

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/205a40a5ac83
Optimize polymorphic Object.is in CacheIR r=jandem
https://hg.mozilla.org/integration/autoland/rev/22fd0e895880
Rename SameValue MIR op r=jandem
https://hg.mozilla.org/integration/autoland/rev/ca806eb90637
Transpile SameValueResult r=jandem

Comment 5

[Iulian Moraru]

Backed out for causing multiple CacheIRCompiler failures.

Push with failures

Failure log
Failure log
Failure log

Backout link

Comment 6

[Iain Ireland [:iain]]

Attachment: Bug 1709216: Add copyToScratchValueRegister to fix x86 r=jandem

useFixedValueRegister was unused (since bug 1673553 part 94, I think), so I cannibalized it for spare parts.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d10063d1349d
Optimize polymorphic Object.is in CacheIR r=jandem
https://hg.mozilla.org/integration/autoland/rev/31d61c8494d0
Rename SameValue MIR op r=jandem
https://hg.mozilla.org/integration/autoland/rev/e7c46af18072
Transpile SameValueResult r=jandem
https://hg.mozilla.org/integration/autoland/rev/3ed6ba663393
Add copyToScratchValueRegister to fix x86 r=jandem

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/d10063d1349d
https://hg.mozilla.org/mozilla-central/rev/31d61c8494d0
https://hg.mozilla.org/mozilla-central/rev/e7c46af18072
https://hg.mozilla.org/mozilla-central/rev/3ed6ba663393