Last Comment Bug 693479 - Remove JSObject::privateData
: Remove JSObject::privateData
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Brian Hackett (:bhackett)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: ObjectShrink
  Show dependency treegraph
 
Reported: 2011-10-10 16:08 PDT by Brian Hackett (:bhackett)
Modified: 2011-12-07 16:46 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (58.68 KB, patch)
2011-10-10 17:17 PDT, Brian Hackett (:bhackett)
luke: review+
Details | Diff | Splinter Review

Description Brian Hackett (:bhackett) 2011-10-10 16:08:24 PDT
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 2 Luke Wagner [:luke] 2011-10-17 14:44:45 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.