1939427 - Demo at https://nischayv.github.io/as-benchmarks/graphics/fire/dist/index.html is 5x slower in Nightly

Status: NEW

(Core :: JavaScript Engine, task, P3)

Description

[Mayank Bansal]

Go to https://nischayv.github.io/as-benchmarks/graphics/fire/dist/index.html
Select "javascript"

Nightly: https://share.firefox.dev/40bns2y (35FPS)
Chrome: https://share.firefox.dev/3DBUlg2 (160FPS)

Comment 1

[Jan de Mooij [:jandem]]

This has a ton of time under IonSetPropertyIC::update, doing something with typed arrays. Maybe double or negative int32 keys again? NI myself to take a quick look at that.

Comment 2

[André Bargull [:anba]]

ICs for TypedArrays handle both double and negative int32 keys. The benchmark has a TypedArray out-of-bounds read for fire[a], which returns undefined. Then there's an addition with generates a BinaryCache, because the right-hand side is either a Number or undefined. When the rhs is undefined, the addition returns NaN. The NaN is propagated through the division and the Math.floor call. (Source: https://github.com/nischayv/as-benchmarks/blob/master/graphics/fire/assembly/index.ts)

Generated MIR:

...
174 loadtypedarrayelementhole arraybufferviewelements161:Elements int32tointptr173:IntPtr arraybufferviewlength158:IntPtr Value
545 keepaliveobject guardshape156:Object
175 box add172:Int32 Value
176 binarycache box175:Value loadtypedarrayelementhole174:Value Value
177 unbox binarycache176 to Double (fallible) Double
178 div unbox177:Double constant114:Double [double] Double
180 guardspecificfunction loaddynamicslotandunbox538:Object constant124:Object Object
181 nearbyint div178:Double (down) Double
182 setpropertycache loadfixedslotandunbox536:Object lsh128:Int32 nearbyint181:Double
...

The final SetPropertyCache is generated because the CacheIR implementation for GuardToInt32ModUint32 doesn't perform recovery when branchTruncateDoubleMaybeModUint32 fails.

Simple test case which also generates SetPropertyCache:

function test() {
  var ta = new Uint8Array(2);
  for (var i = 0; i < 1000000; ++i) {
    ta[i & 1] = NaN;
  }
  return ta[0] + ta[1];
}

Comment 3

[Jan de Mooij [:jandem]]

Thanks for the analysis! Could we use the same callWithABI as in CacheIRCompiler::emitTruncateDoubleToUInt32?

Comment 4

[André Bargull [:anba]]

Bug 1942255 should make this ~2.3x times faster. It's still slower than JSC/V8 because the BinaryCache will prevent moving guards before the loops.

Comment 5

[Mayank Bansal]

With the latest Nightly: https://share.firefox.dev/4hgbHh8 (120fps without the profiler)