Closed Bug 762421 Opened 12 years ago Closed 12 years ago

IonMonkey: Implement Name ICs

Categories

(Core :: JavaScript Engine, defect)

All
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch WIP v0 (obsolete) — Splinter Review
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.
Attached patch a patchSplinter Review
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
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.