1133254 - Improve type information and Ion compilation when dealing with converted unboxed objects

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

Currently, converting an unboxed object to its native representation destroys type information for the old and new types of the object, and incurs various performance-hurting deoptimizations (mainly type barriers and shape guards).  The attached patch improves this situation by preserving type information and allowing Ion to generate optimized fixed slot and monomorphic/polymorphic property accesses, by inserting instructions to eagerly convert incoming unboxed objects to native objects.  On the following benchmark, which has monomorphic accesses on objects that might be converted:

function Foo(a, b) {
    this.a = a;
    this.b = b;
}
arr = [];
for (var i = 0; i < 100; i++)
    arr.push(new Foo(i, i + 1));
arr[0].c = 3;

function iter() {
    var total = 0;
    for (var i = 0; i < 1000000; i++) {
        for (var j = 0; j < arr.length; j++)
            total += arr[j].a + arr[j].b;
    }
    return total;
}
iter();

I get these times (x86 10.9):

without unboxed objects:  126 ms
unboxed objects (old):    295 ms
unboxed objects (new):    154 ms

If I modify the benchmark by adding 'arr.push({a:0,b:1});' before iter(), so that it uses an inlined polymorphic access:

without unboxed objects:  160 ms
unboxed objects (old):    301 ms
unboxed objects (new):    280 ms

The main difference between 'without unboxed objects' and 'unboxed objects (new)' is due to the fact that the converted objects have a different number of fixed slots than they would have if they were originally created as natives.  This particularly hurts on the second test variant because we need to use GetPropertyPolymorphicT instead of GuardShapePolymorphic (which can be consolidated) and LoadFixedSlotT.

These benchmarks are totally artificial of course, and this patch also doesn't fix box2d, the only octane test which converts unboxed objects and has a ~5% regression with unboxed objects turned on (x86 10.9).  I'll be looking more at this test later.  All other octane tests are at least as fast with unboxed objects turned on.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: cleanup

Make some cleanup fixes for the rest of the patch.  Lots of NativeObject methods related to changing the last property are handlified, which is unnecessary as these methods can't GC, and makes things more of a pain for callers.  This also changes JSObject::setFlag to JSObject::setFlags and lets it set more than one flag at a time.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: main

The main part of the patch.

Comment 3

[Terrence Cole [:terrence]]

Comment on attachment 8564608 [details] [diff] [review]
cleanup

Review of attachment 8564608 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

Comment 4

[Brian Hackett [Laid off!]]

Part 1:

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1cc2dd3e35c

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8564609 [details] [diff] [review]
main

Review of attachment 8564609 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b1cc2dd3e35c

Comment 7

[Brian Hackett [Laid off!]]

Part 2:

https://hg.mozilla.org/integration/mozilla-inbound/rev/456afb8e4650

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/456afb8e4650