Closed Bug 655907 Opened 9 years ago Closed 8 years ago

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


(Core :: JavaScript Engine, defect)

Not set





(Reporter: Waldo, Assigned: Waldo)


(Depends on 2 open bugs)



(1 file, 1 obsolete file)

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