Closed Bug 684507 Opened 9 years ago Closed 8 years ago

Remove JSObject::isNewborn()


(Core :: JavaScript Engine, defect)

Not set





(Reporter: bhackett1024, Assigned: bhackett1024)




(1 file)

Every time the GC marks an object it first needs to check isNewborn() (lastProp != NULL), which if false means the rest of the object is garbage.  This is because places where objects are created can trigger a GC after having allocated the object but before it is filled in, and these places are required to fill in lastProp last.  It would be simpler and faster to require these places to assemble the contents of the object (shape, type, parent) first, allocate the object and fill it in entirely without any potential for a GC when it is half-initialized.
Attached patch patchSplinter Review
Restructure object creation so that the type, shape, and necessary slots are allocated up front and the object is filled to a valid state immediately after creation.  This is basically a rewrite of the object creation code.  Luke, sorry for saddling you with all these object shrink reviews, let me know if you want me to hand any off to other folks.

Background: the creation path cleanup is necessary because creating objects in the VM is currently much slower in the JM branch than on trunk due to the various hashtable lookups involved.  This funnels things better and will allow making the VM creation path fast again (faster than trunk) through the use of a couple caches.
Assignee: general → bhackett1024
Attachment #571437 - Flags: review?(luke)
Blocks: ObjectShrink
Comment on attachment 571437 [details] [diff] [review]

Review of attachment 571437 [details] [diff] [review]:

A most righteous patch!

::: js/src/jsfun.cpp
@@ +155,5 @@
> +    JS_STATIC_ASSERT(StrictArgumentsObject::RESERVED_SLOTS == 2);
> +    JSObject *obj = js_NewGCObject(cx, FINALIZE_OBJECT4);
> +    if (!obj)
> +        return NULL;
> +    obj->initialize(emptyArgumentsShape, type, NULL);

Further reaping the rewards of your work: could you make a static JSObject::create to factor out the js_NewGCObject+initialize pattern (thereby allowing your to remove initialize())?

::: js/src/jsobjinlines.h
@@ +937,5 @@
> +        setPrivate(NULL);
> +
> +    size_t span = shape->slotSpan();
> +    if (span)
> +        clearSlotRange(0, span);

if (size_t span = ...)

@@ +1604,3 @@
>  {
> +    size_t count = JSObject::dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan());
> +    if (count) {

if (size_t count = ...)

Also, can we name this AllocateObjectDynamicSlots?
Attachment #571437 - Flags: review?(luke) → review+
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.