832360 - Make an object's type determine its class

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Storing an object's class inside its type instead of its base shape has two important advantages.

- Would simplify a lot of JIT optimization, as since objects with the same type can have different classes we need to track state in the type about the various kinds of classes.  This is needed even more going forward (DOM accesses through proxies, dense element accesses on native objects, ...).

- Would cut a load when accessing an object's class. (obj->type->clasp instead of obj->shape->base->clasp)

On 32 bit platforms TypeObject has an unused word, so this change won't affect memory usage there (and only slightly on 64 bit, as BaseShape loses a word on that platform).

With bug 827490 done I do not think there are any points where an object's class changes, except in brain transplants.  Though during brain transplants an object's prototype also changes (which is also determined by the type) so there shouldn't be any new complexity in that case.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: patch

Patch.  This doesn't remove the clasp from BaseShape, but just additionally adds it to TypeObject.  I'd like to remove it from the BaseShape but doing so requires more refactoring in the shape code (the clasp is used for things like determining the slot span and keying cache entries) and is better off separated.

Comment 2

[Jan de Mooij [:jandem]]

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

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

Looks good, r=me with comments below addressed.

Can you rewrite loadObjClass and branchTestObjClass in IonMacroAssembler to load the class from the type?

::: js/src/ion/IonBuilder.cpp
@@ +5815,5 @@
>              if (propSet->ownProperty(false))
>                  return true;
>  
>              // Check the DOM status of the instance type
> +            thinkDOM = thinkDOM && (typeObj->flags & JSCLASS_IS_DOMJSCLASS);

typeObj->clasp->flags

Even better if we could just do "*isDOM = types->isDOMClass()" at the end of this method instead.

@@ +5828,5 @@
>                  return true;
>  
>              // Check the DOM-ness of the singleton.
>              types::TypeObject *objType = curObj->getType(cx);
> +            thinkDOM = thinkDOM && (objType->flags & JSCLASS_IS_DOMJSCLASS);

Same here.

::: js/src/jsinfer.cpp
@@ +2014,5 @@
> +    Class *clasp = getKnownClass();
> +
> +    if (clasp && IsTypedArrayClass(clasp))
> +        return clasp - &TypedArray::classes[0];
> +    return TypedArray::TYPE_MAX;

Nice.

::: js/src/methodjit/LoopState.cpp
@@ +787,5 @@
>      if (!loopInvariantEntry(objSlot))
>          return NULL;
>  
>      /* Hoist 'length' access on typed arrays. */
> +    if (!objTypes->getTypedArrayType() != TypedArray::TYPE_MAX) {

Remove ! before objTypes

Comment 3

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/397dbf58a6ff

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/397dbf58a6ff