Bug 22482 – REGRESSION (r37991): Occasionally see "Scene rendered incorrectly" message when running the V8 Raytrace benchmark

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Cameron Zwarich (cpst)]

When running the V8 Raytrace benchmark, many of us have seen the "Scene rendered incorrectly" message. It doesn't always occur, so I have always suspected that it is a GC issue. If I hack up the V8 test harness, it always occurs with the JIT and COLLECT_ON_EVERY_ALLOCATION. It does not occur with the bytecode interpreter, even with COLLECT_ON_EVERY_ALLOCATION.

I will try to reduce and fix this. I suspect that it is caused by some bad interaction between inline caching and GC.

[Cameron Zwarich (cpst)]

Here is a reduction for the jsc shell, only with COLLECT_ON_EVERY_ALLOCATION:

var o = {
    x: 0.2,

    normalize: function() {
        return this.x / 5.0039984012787215;
    }
};

print(-o.normalize());

This prints 0, but it should print something nonzero. I believe that it happens because a number cell is being collected. I can't trigger it by putting explicit gc() calls anywhere.

[Cameron Zwarich (cpst)]

The problem seems to be caused by the negation, as it goes away if I remove it. The GC caused by the call to allocateNumber() emitted by

                if (!resultType.isReusableNumber())
                    emitAllocateNumber(m_globalData, i);

must not be marking the operand to op_negate.

[Cameron Zwarich (cpst)]

Here is a simpler reduction:

function f()
{
    return 0.1;
}

print(-f());

[Alexey Proskuryakov]

Created attachment 25478 [details]
in-browser test case

Same as the above, but doesn't need COLLECT_ON_EVERY_ALLOCATION.

[Cameron Zwarich (cpst)]

This is likely caused by r37991:

http://trac.webkit.org/changeset/37991

Oliver says that he probably has a fix.

[Cameron Zwarich (cpst)]

I have verified that this is caused by r37991.

[Cameron Zwarich (cpst)]

Oliver indeed has a fix for this, but it is part of other work. The problem is that xmm0 is not callee-save, so eventually some function (like memcpy()) will get called and smash its value. I am assigning this to Oliver.

[Cameron Zwarich (cpst)]

Created attachment 25614 [details]
Patch to fix problem

This patch simply reverts r37991. I can't accurately test its performance. It seems to be a regression, but my loaner machine has insane variance.

[Geoffrey Garen]

Comment on attachment 25614 [details]
Patch to fix problem

r=me

Performance seems OK.

[Geoffrey Garen]

Committed revision 38917.