1801794 - Js1k demo (https://js1k.com/2017-magic/demo/2900) doesnt complete loading, and slows down the whole browser

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect, P3)

Description

[Mayank Bansal]

Go to https://js1k.com/2017-magic/demo/2900

ER: Page loads
AR: Page keeps on loading. A message saying "this tab is slowing your browser" appears. Chrome is able to load the page

This also repros on a build from Nov-2017. So this is not a recent regression.

Comment 1

[Mayank Bansal]

Unsure if this is a Canvas bug, or JS or something else.

Comment 2

[Mayank Bansal]

Attachment: about:support

Comment 3

[Mayank Bansal]

Attachment: Standalone_testcase.htm

Just a standalone version of the page. Not much minimized.

Comment 4

[Mayank Bansal]

The regression range is [2015-12-27, 2015-12-28]

Comment 5

[Mayank Bansal]

This is a highly obfuscated testcase, so there is probably not much use in "fixing" this. Feel free to WONTFIX etc.

Comment 6

[Nicolas B. Pierron [:nbp]]

I see the same problem in both Chromium and Firefox, and Steven see the problem only in Chromium but not in Firefox …

I do not see any JS related changed on Monday 28th 2015, no patches on Sunday 27th.
However, we have JIT patches on Saturday 26th / Tuesday 29th.

https://hg.mozilla.org/integration/autoland/pushloghtml?startdate=2015-12-25&enddate=2015-12-30

On the other hand, looking at a profile:

• 16% of the time is spent in mozilla::detail::EntrySlot<js::WeakHeapPtr<Atom*> const>::isFree() const, when calling Ion IC (js::jit::IonGetPropertyIC::update) with a numerical value which is atomized (js::Int32ToAtom).

Here is the (reverse) stack trace:

js::jit::IonGetPropertyIC::update(JSContext*, JS::Handle<JSScript*>, js::jit::IonGetPropertyIC*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)
js::GetElementOperation(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)
js::GetElementOperationWithStackIndex(JSContext*, JS::Handle<JS::Value>, int, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)
js::PrimitiveValueToIdSlow<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::MutableHandleType)
PrimitiveToAtom<(js::AllowGC)1>(JSContext*, JS::Value const&)
js::Int32ToAtom(JSContext*, int)
js::Atomize(JSContext*, char const*, unsigned long, mozilla::Maybe<unsigned int> const&)
AtomizeAndCopyChars<unsigned char>(JSContext*, unsigned char const*, unsigned long, mozilla::Maybe<unsigned int> const&)
AtomizeAndCopyCharsNonStaticValidLengthFromLookup<unsigned char>(JSContext*, unsigned char const*, unsigned long, js::AtomHasher::Lookup const&, mozilla::Maybe<unsigned int> const&)
mozilla::HashSet<js::WeakHeapPtr<JSAtom*>, js::AtomHasher, js::SystemAllocPolicy>::lookupForAdd(js::AtomHasher const::Lookup&)
mozilla::detail::HashTable<js::WeakHeapPtr<JSAtom*> const, mozilla::HashSet<js::WeakHeapPtr<JSAtom*>, js::AtomHasher, js::SystemAllocPolicy>::SetHashPolicy, js::SystemAllocPolicy>::lookupForAdd(js::AtomHasher const::Lookup&)
mozilla::detail::HashTable<js::WeakHeapPtr<JSAtom*> const, mozilla::HashSet<js::WeakHeapPtr<JSAtom*>, js::AtomHasher, js::SystemAllocPolicy>::SetHashPolicy, js::SystemAllocPolicy>::lookup<(mozilla::detail::HashTable<js::WeakHeapPtr<JSAtom*> const, mozilla::HashSet<js::WeakHeapPtr<JSAtom*>, js::AtomHasher, js::SystemAllocPolicy>::SetHashPolicy, js::SystemAllocPolicy>::LookupReason)1>(js::AtomHasher const::Lookup&, unsigned int) const
mozilla::detail::EntrySlot<js::WeakHeapPtr<JSAtom*> const>::isFree() const

• 14% is the function f located at https://js1k.com/2017-magic/demo/2900 line 113 > injectedScript:2:10

One question is what is this injectedScript displayed in the profiler? I see it in Mozilla code base as an introductionType, but I do not know whether this is related or not.

Iain, Jon, would there be anything obvious to be done around EntrySlot<WeakHeapPtr<JSAtom*>>::isFree / Int32ToAtom case?

Comment 7

[Iain Ireland [:iain]]

Looking at PrimitiveValueToId, we only call PrimitiveValueToIdSlow for an Int32 if the index is negative. So presumably this code is using a lot of negative indices.

I took a look at the source code, which I think has been minified by hand as a demo. Pulling out the core code and adding enough definitions to get it to run, I get this:

var c = {
  fillRect: function() {},
  clearRect: function() {},
  beginPath: function() {},
  lineTo: function() {},
  fill: function() {},
}

var a = {
  width: 15, // **** THIS IS THE KEY ****
  height: 15,
  style: {
    background: undefined
  }
}

var count = 0;
function requestAnimationFrame(f) {
  if (count++ < 100) f(count * 2000)
}
// everything below this line is original

function f(a) {
    for (a /= 1e3, c.clearRect(0, 0, W, H), c.fillStyle = "#FFF", o = ~~(a / (M > 1 ? 9 : 2) * W) % W, Z[0] = [], i = 1; i < 4; ++i)
        for (j = 0; j < W; ++j) k = j - o, k < 0 && (k += W), x = k / i | 0, y = Z[i][j] / i, Z[0][x] < y || (Z[0][x] = y);
  for (i = W; i;) {
        if (!(M % 2)) {
          for (X = [], Y = [], z = l = 0, j = 3; j--; i--) k = i - (M > 1 ? o : 0), k < 0 && (k += W), k && (X[j] = k, Y[j] = Math.tan(i % 99 * i + a) * H / 8 + H / 8, Y[j] > z && (z = Y[l = j]));
          Y[l] > Z[0][i] && (Y[l] = Z[0][i])

        }
        for (c.beginPath(), j = 3; j--;) M % 2 ? (k = i - (M > 1 ? o : 0), k < 0 && (k += W), k && (z = Math.tan(i % 99 * i + a) * H / 8 + H / 8, z > Z[0][i] && (c.fillRect(k, z, 2, 2), z = Z[0][i]), c.lineTo(k, z)), i--) : c.lineTo(X[j], Y[j]);
        c.fill()
    }
    requestAnimationFrame(f)
}
for (R = Math.random, W = a.width, H = a.height, a.style.background = "#000", Z = [], i = 1; i < 4; ++i)
    for (Z[i] = [], x = y = 0, j = 0; j < W; ++j) j == x && (x += R() * W / 8 + W / 8 | 0, y = y ? 0 : R() * H / 4 + H / 4), Z[i][j] = H - y;
M = 0, a.onclick = function() {
    ++M, M > 3 && (M = 0)
}, f();

I didn't immediately see where the negative indices were being used, so I tried running it. Interestingly, although the original webpage works fine for me, this code got stuck in an infinite loop. After debugging it for a bit, I realized that i was becoming increasingly negative in the for (i = W; i; ) loop. The values of i were decreasing by 3 each iteration. By setting the initial value of a.width to a multiple of 3, everything worked out.

It looks like a is initialized here: k=g.createElement("iframe");g.body.appendChild(k);var a=k.contentWindow. So it's the width of the iframe. If it's a multiple of 3, then this code works. Otherwise, it gets caught in an infinite loop.

Looking at the webpage in devtools, the canvas width is 1920, which is also the horizontal resolution of my laptop display. If I temporarily change the resolution to 1600x900, which is not divisible by 3, the demo hangs.

So the bug appears to be in the webpage, not Firefox.

Comment 8

[Mayank Bansal]

Great investigation!
Resolving as INVALID.