1790791 - Optimize megamorphic SetProp/SetElem for PlainObject

Status: RESOLVED FIXED

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

Description

[Jan de Mooij [:jandem]]

We've optimized megamorphic GetProp and HasProp pretty well, but SetProp/SetElem often shows up in profiles because we go through SetObjectElementWithReceiver => NativeSetProperty. This is the generic set-property code and it's relatively slow.

In practice, almost all of these megamorphic sets are setting/adding a string property on a PlainObject and we can carve out a fast path for just this case. I wrote a patch for this and collected some data:

matrix-react benchmark:

Optimized (PlainObject):    94.2%
Not optimized (other):       3.3%
Not optimized (PlainObject): 2.5%

Speedometer 2:

Optimized (PlainObject):    98.0%
Not optimized (other):       2.0%
Not optimized (PlainObject): 0.0% (actually 0.027%)

The JS shell micro-benchmark below improves from ~364 ms to ~211 ms.

function f() {
    var t = new Date;
    var props = ["x1", "x2", "x3", "x4", "x5", "x6", "x7",
                 "x3", "x5", "x8", "x1"];
    var o;
    for (var i = 0; i < 1_000_000; i++) {
        o = {};
        for (var j = 0; j < props.length; j++) {
            o[props[j]] = j;
        }
    }
    print(new Date - t);
    return o;
}
f();

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1790791 part 1 - Rename MCallSetElement to MMegamorphicSetElement. r?iain!

This matches the name of the corresponding CacheIR instruction.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1790791 part 2 - Add js::jit::SetElementMegamorphic. r?iain!

Depends on D157330

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1790791 part 3 - Add a fast path for megamorphic set/add on plain objects. r?iain!

More than 94% of calls to SetElementMegamorphic on matrix-react-bench, Speedometer 2,
and a number of popular websites can be optimized with this fast path.

This makes a micro-benchmark 1.72x faster and should eliminate most of the overhead,
but we could potentially optimize this more in the future with the megamorphic
cache.

Depends on D157331

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/548e62dfe628
part 1 - Rename MCallSetElement to MMegamorphicSetElement. r=iain
https://hg.mozilla.org/integration/autoland/rev/a9c7e9eba6b8
part 2 - Add js::jit::SetElementMegamorphic. r=iain
https://hg.mozilla.org/integration/autoland/rev/900845a8e813
part 3 - Add a fast path for megamorphic set/add on plain objects. r=iain

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/548e62dfe628
https://hg.mozilla.org/mozilla-central/rev/a9c7e9eba6b8
https://hg.mozilla.org/mozilla-central/rev/900845a8e813

Comment 6

[Mayank Bansal]

Some nice improvements on AWFY Speedometer:
Elm-TodoMVC/*
React-Redux-TodoMVC*