The default bug view has changed. See this FAQ.

Start homing class-prototype lookups directly in the global object, not in a windy series of steps beneath js_GetClassPrototype

RESOLVED FIXED in mozilla10

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

(Depends on: 2 bugs)

Trunk
mozilla10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 531212 [details] [diff] [review]
Patch

js_GetClassPrototype is overly complex.  It can more or less be replaced with direct accessor methods on the global object that return the value from the appropriate slot.  Get those methods in place, and use them in those places where it's easy to do so.  (That's not all places, currently, because some code and APIs assume dynamic-scoping-like functionality that looks at the call stack to figure it out.  And the call stack is a lie -- bug 631135 -- in some cases, so making it new-style while still preserving old-style misbehavior until a full fix is entertained is somewhat tricky.  For now I'm leaving all such places alone, if the NULL parent passed to js_GetClassPrototype couldn't easily be replaced a fp->scopeChain() or similar.)

Some of these replacements doubtless could use infallible methods because if they're called, they *know* they've been called after the relevant class has been created, or whatever.  But the case-by-case analysis needed to get that right each time an optimization appears possible seems error-prone.  Further, it's very little overhead to check a slot value for being |undefined| or not.  So for now I'm not worrying about it.  When standard classes come with the global object at construction time, then we can switch every use to be infallible.

This builds on the patch stack in bug 655192, of course.
Attachment #531212 - Flags: review?(jorendorff)
Blocks: 655921
Depends on: 656043
Created attachment 531802 [details] [diff] [review]
Don't change js::FindProto

Oops, shouldn't have changed FindProto, that's obviously not possible given the parent passed to it could be NULL.
Attachment #531212 - Attachment is obsolete: true
Attachment #531212 - Flags: review?(jorendorff)
Attachment #531802 - Flags: review?(jorendorff)
Comment on attachment 531802 [details] [diff] [review]
Don't change js::FindProto

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

I think it's a little more code total with the patch than without it, but I am going to enjoy writing getOrCreateFunctionPrototype() from now on, instead of searching the codebase for code to crib. r=me.

::: js/src/jsiter.cpp
@@ +1452,5 @@
>  {
>      /* Create and initialize Iterator.prototype. */
> +    JSObject *objectProto = global->getOrCreateObjectPrototype(cx);
> +    if (!objectProto)
> +        return NULL;

return false; here.

::: js/src/methodjit/Compiler.cpp
@@ -3242,5 @@
> -    /*
> -     * Bake in String.prototype. This is safe because of compileAndGo.
> -     * We must pass an explicit scope chain only because JSD calls into
> -     * here via the recompiler with a dummy context, and we need to use
> -     * the global object for the script we are now compiling.

Good deleting.
Attachment #531802 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/bdd89e099e60
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/bdd89e099e60
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.