569537 - TM: Rip out proto cache and store prototypes in the global object

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

We cache class objects in the global object. A second cache in the context was added on top of this to cache the prototype. This can be done simpler by caching the prototype in the global object alongside the class object. Also add a fast path to access the reserved slots of the global object, which we know to be single-threaded (no need to lock).

Comment 1

[Andreas Gal :gal]

Attachment: patch

Comment 2

[Brendan Eich [:brendan]]

David, did you double the global reserved slots for another reason in JM?

/be

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to comment #2)

It's to force predictable slots in the parser when resolving, without clobbering the original ones cached in the first set of JSProto keys.

Comment 4

[Andreas Gal :gal]

Attachment: patch

Comment 5

[Jason Orendorff [:jorendorff]]

Comment on attachment 448932 [details] [diff] [review]
patch

This changes the engine to assert that the global object always has JSCLASS_GLOBAL_FLAGS. I think this is a good idea and now is as good a time as any, but please add assertions in JS_SetGlobalObject, JS_InitStandardClasses, and JS_ResolveStandardClass to detect this as early as possible.

Comment 6

[Andreas Gal :gal]

We agreed to just fall back on the slow path here instead of an assert since AnyName is passed in as scope when making it's functions.

Comment 7

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/0a5778b1355b

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/0a5778b1355b

Comment 9

[Brendan Eich [:brendan]]

Comment on attachment 448932 [details] [diff] [review]
patch

>+++ b/js/src/jsapi.h
>@@ -1519,17 +1519,17 @@ struct JSExtendedClass {
>  * in ECMA-262, global properties naming class objects are read/write and
>  * deleteable, for the most part.
>  *
>  * Implementing this efficiently requires that global objects have classes
>  * with the following flags. Failure to use JSCLASS_GLOBAL_FLAGS was
>  * prevously allowed, but is now an ES5 violation and thus unsupported.
>  */
> #define JSCLASS_GLOBAL_FLAGS \
>-    (JSCLASS_IS_GLOBAL | JSCLASS_HAS_RESERVED_SLOTS(JSProto_LIMIT * 2))
>+    (JSCLASS_IS_GLOBAL | JSCLASS_HAS_RESERVED_SLOTS(JSProto_LIMIT * 3))

Why 3 "banks" of slots indexed by JSProto_* key, offset by bank-start index? No comment says, and I see no reason. More below.

>@@ -3413,17 +3413,17 @@ DefineStandardSlot(JSContext *cx, JSObje
>         JSScope *scope = js_GetMutableScope(cx, obj);
>         if (!scope) {
>             JS_UNLOCK_OBJ(cx, obj);
>             return false;
>         }
> 
>         JSScopeProperty *sprop = scope->lookup(id);
>         if (!sprop) {
>-            uint32 index = JSProto_LIMIT + key;
>+            uint32 index = 2 * JSProto_LIMIT + key;
>             if (!js_SetReservedSlot(cx, obj, index, v)) {

Here we store obj at key slots into the "third bank".

DefineStandardSlot is called only from js_InitClass, passing as v either the singleton class object (e.g., Math, for the !constructor case), or the class constructor function (tvr2.value() -- thought we were done with tvrs, hmph).

Neither of these is the class prototype object for the case where there is a constructor function for the class. But class protos are stored in the "second bank", so why did this patch add a third bank anyway?

>     /* If this is a standard class, cache its prototype. */
>-    if (key != JSProto_Null && !js_SetClassObject(cx, obj, key, ctor))
>+    if (key != JSProto_Null && !js_SetClassObject(cx, obj, key, ctor, proto))

Ok, so where is js_SetClassObject?

>+js_SetClassObject(JSContext *cx, JSObject *obj, JSProtoKey key, JSObject *cobj, JSObject *proto)
> {
>     JS_ASSERT(!obj->getParent());
>     if (!(obj->getClass()->flags & JSCLASS_IS_GLOBAL))
>         return JS_TRUE;
> 
>-    return JS_SetReservedSlot(cx, obj, key, OBJECT_TO_JSVAL(cobj));
>+    return js_SetReservedSlot(cx, obj, key, OBJECT_TO_JSVAL(cobj)) &&
>+           js_SetReservedSlot(cx, obj, JSProto_LIMIT + key, OBJECT_TO_JSVAL(proto));

cobj (class object, constructor typically) goes in first bank, and class proto goes in second bank. Nothing in third bank, and no code ever gets a third-bank slot.

Andreas or Jason, could one of you please file a bug to remove this third bank and simplify accordingly, unless I've missed a need for it somewhere else?

/be

Comment 10

[Brendan Eich [:brendan]]

Filed bug 594221.

/be