693479 - Remove JSObject::privateData

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Most created objects don't have private data, especially now that bug 693221 has removed the need for JSCLASS_HAS_PRIVATE in the array classes.  Instead of wasting space for privateData in every object, a fixed slot can be removed from every object with private data (as before, this value is not scanned by the GC, so does not need to be a private pointer).  The last fixed slot is removed, to avoid mucking with computations of the object's base fixed slot address and GC marking.  This requires checking numFixedSlots() when accessing the private data, but in many cases the number of fixed slots is invariant for particular classes and can be baked into the VM.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: patch

https://hg.mozilla.org/projects/jaegermonkey/rev/07c668448519

Comment 2

[Luke Wagner [:luke]]

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

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

::: js/src/jsgcinlines.h
@@ +408,5 @@
>  js_NewGCFunction(JSContext *cx)
>  {
>      JSFunction *fun = NewGCThing<JSFunction>(cx, js::gc::FINALIZE_FUNCTION, sizeof(JSFunction));
>      if (fun)
> +        fun->earlyInit(JSObject::FUN_CLASS_NFIXED_SLOTS + 1);  /* Add one for private data. */

Blech, but I see this is removed by subsequent patches so whatever.

::: js/src/jsobj.h
@@ +892,2 @@
>      inline void *getPrivate() const;
> +    inline void *getPrivate(size_t nfixed) const;

This is scary as a public interface.  Ideally, I think it would be protected and then specific object types would provide obj->asX().getPrivate() which called the protected getPrivate, passing the right constant.  I see the only use is TypedArray::getDataOffset. Since typed arrays don't have a JSObject subclass, you could just make a JSObject::getDataOffset() called the protected getPrivate.

::: js/src/jsobjinlines.h
@@ +99,5 @@
> +    return getClass()->flags & JSCLASS_HAS_PRIVATE;
> +}
> +
> +inline void *&
> +JSObject::privateAddress(uint32 nfixed) const

Could this be privateRef instead?  The name implies a void ** retval.

@@ -938,5 @@
>      uint32 numSlots = numFixedSlots();
>  
>      /*
>       * Fill the fixed slots with undefined if needed.  This object must
>       * already have its numFixedSlots() filled in, as by js_NewGCObject.

Comment needs a bit of freshening since neither arrays nor objects init fixed slots here.  Mentioning where init() fits in the big picture of object construction would be nice.