IonMonkey: Implement Name ICs

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Though we have the fancy new ALIASEDVAR opcodes, they don't reach across function boundaries yet. Name ICs will let us optimize these accesses (and others) until the front-end is changed. When that happens (bug 753158), most of the really hot NAME ops will become ALIASEDVAR, but the name ICs will still be useful for eval-poisoned scopes.

This bug will should get us most of the way there for JSOP_NAME; bug 753158 will be an additional win since the steps emitted by the ICs will instead be inlined as MIR.
Created attachment 631246 [details] [diff] [review]
WIP v0

Benchmark: 
var f = function (x, y) {
    return (function () {
        var t;
        for (var i = 0; i < 10000000; i++)
            t = y;
        return t;
    })();
}

Goes from 1054ms to 27ms. Yay!

Otherwise untested, test tomorrow.
Created attachment 632090 [details] [diff] [review]
a patch
Attachment #631246 - Attachment is obsolete: true
Attachment #632090 - Flags: review?(jdemooij)
Comment on attachment 632090 [details] [diff] [review]
a patch

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

Looks good, r=me with comments addressed.

::: js/src/ion/IonCaches.cpp
@@ +1107,5 @@
> +        if (!IsCacheableNonGlobalScope(obj2))
> +            return false;
> +
> +        // Stop once we hit the global or target obj.
> +        if (obj2->isGlobal() || obj2 == obj)

IsCacheableNonGlobalScope will return |false| if obj2->isGlobal(). Do we need something like "&& !obj2->isGlobal()" there to allow global properties?

It would be good if we could somehow call IsCacheableScopeChain here if possible.

@@ +1138,5 @@
> +
> +    if (cache.stubCount() < MAX_STUBS &&
> +        IsCachableName(cx, scopeChain, obj, holder, prop))
> +    {
> +        if (!cache.attach(cx, scopeChain, obj, (Shape *)prop))

A cache.incrementStubCount(); here to avoid adding too many stubs.
Attachment #632090 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/6dd068259832
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.