Closed Bug 832360 Opened 7 years ago Closed 7 years ago

Make an object's type determine its class


(Core :: JavaScript Engine, defect)

Other Branch
Not set





(Reporter: bhackett, Assigned: bhackett)




(1 file)

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.
Blocks: 832364
Attached patch patchSplinter Review
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.
Assignee: general → bhackett1024
Attachment #704131 - Flags: review?(jdemooij)
Comment on attachment 704131 [details] [diff] [review]

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);


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;


::: 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
Attachment #704131 - Flags: review?(jdemooij) → review+
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Blocks: 849014
You need to log in before you can comment on or make changes to this bug.