Bug 1433837 - Cleanup JSObject initialization nits r=jandem
authorTed Campbell <tcampbell@mozilla.com>
Sun, 28 Jan 2018 20:31:00 +0200
changeset 453720 9cbb0f5c50c6b19f00654a169e6eeccc40c37468
parent 453719 794dda2e9f2edd93adc5ce42ebb4447b8e399410
child 453721 b1c8ba93a6c2f5e9179c8ff252ba7e049558e408
push id8799
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 16:46:23 +0000
treeherdermozilla-beta@15334014dc67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1433837
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1433837 - Cleanup JSObject initialization nits r=jandem Make JSObject initializations more consistent accross different types. MozReview-Commit-ID: Ixbr1bfM0hj
js/src/builtin/TypedObject.cpp
js/src/gc/Allocator.cpp
js/src/gc/Nursery.cpp
js/src/jit/MacroAssembler.cpp
js/src/jsobj.h
js/src/jsobjinlines.h
js/src/vm/ArrayObject-inl.h
js/src/vm/Caches-inl.h
js/src/vm/Caches.h
js/src/vm/NativeObject-inl.h
js/src/vm/NativeObject.h
js/src/vm/ProxyObject.cpp
js/src/vm/UnboxedObject.cpp
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -2404,21 +2404,19 @@ TypedObject::create(JSContext* cx, js::g
     const js::Class* clasp = group->clasp();
     MOZ_ASSERT(::IsTypedObjectClass(clasp));
 
     JSObject* obj = js::Allocate<JSObject>(cx, kind, /* nDynamicSlots = */ 0, heap, clasp);
     if (!obj)
         return cx->alreadyReportedOOM();
 
     TypedObject* tobj = static_cast<TypedObject*>(obj);
-    tobj->group_.init(group);
+    tobj->initGroup(group);
     tobj->initShape(shape);
 
-    tobj->setInitialElementsMaybeNonNative(js::emptyObjectElements);
-
     MOZ_ASSERT(clasp->shouldDelayMetadataBuilder());
     cx->compartment()->setObjectPendingMetadata(cx, tobj);
 
     js::gc::TraceCreateObject(tobj);
 
     return tobj;
 }
 
--- a/js/src/gc/Allocator.cpp
+++ b/js/src/gc/Allocator.cpp
@@ -33,17 +33,17 @@ js::Allocate(JSContext* cx, AllocKind ki
     MOZ_ASSERT(IsObjectAllocKind(kind));
     size_t thingSize = Arena::thingSize(kind);
 
     MOZ_ASSERT(thingSize == Arena::thingSize(kind));
     MOZ_ASSERT(thingSize >= sizeof(JSObject_Slots0));
     static_assert(sizeof(JSObject_Slots0) >= MinCellSize,
                   "All allocations must be at least the allocator-imposed minimum size.");
 
-    MOZ_ASSERT_IF(nDynamicSlots != 0, clasp->isNative() || clasp->isProxy());
+    MOZ_ASSERT_IF(nDynamicSlots != 0, clasp->isNative());
 
     // Off-thread alloc cannot trigger GC or make runtime assertions.
     if (cx->helperThread()) {
         JSObject* obj = GCRuntime::tryNewTenuredObject<NoGC>(cx, kind, thingSize, nDynamicSlots);
         if (MOZ_UNLIKELY(allowGC && !obj))
             ReportOutOfMemory(cx);
         return obj;
     }
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -260,35 +260,35 @@ js::Nursery::leaveZealMode() {
         MOZ_ASSERT(isEmpty());
         setCurrentChunk(0);
         setStartPosition();
     }
 }
 #endif // JS_GC_ZEAL
 
 JSObject*
-js::Nursery::allocateObject(JSContext* cx, size_t size, size_t numDynamic, const js::Class* clasp)
+js::Nursery::allocateObject(JSContext* cx, size_t size, size_t nDynamicSlots, const js::Class* clasp)
 {
     /* Ensure there's enough space to replace the contents with a RelocationOverlay. */
     MOZ_ASSERT(size >= sizeof(RelocationOverlay));
 
     /* Sanity check the finalizer. */
     MOZ_ASSERT_IF(clasp->hasFinalize(), CanNurseryAllocateFinalizedClass(clasp) ||
                                         clasp->isProxy());
 
     /* Make the object allocation. */
     JSObject* obj = static_cast<JSObject*>(allocate(size));
     if (!obj)
         return nullptr;
 
     /* If we want external slots, add them. */
     HeapSlot* slots = nullptr;
-    if (numDynamic) {
+    if (nDynamicSlots) {
         MOZ_ASSERT(clasp->isNative());
-        slots = static_cast<HeapSlot*>(allocateBuffer(cx->zone(), numDynamic * sizeof(HeapSlot)));
+        slots = static_cast<HeapSlot*>(allocateBuffer(cx->zone(), nDynamicSlots * sizeof(HeapSlot)));
         if (!slots) {
             /*
              * It is safe to leave the allocated object uninitialized, since we
              * do not visit unallocated things in the nursery.
              */
             return nullptr;
         }
     }
--- a/js/src/jit/MacroAssembler.cpp
+++ b/js/src/jit/MacroAssembler.cpp
@@ -1200,18 +1200,18 @@ MacroAssembler::initGCSlots(Register obj
 void
 MacroAssembler::initGCThing(Register obj, Register temp, JSObject* templateObj,
                             bool initContents, bool convertDoubleElements)
 {
     // Fast initialization of an empty object returned by allocateObject().
 
     storePtr(ImmGCPtr(templateObj->group()), Address(obj, JSObject::offsetOfGroup()));
 
-    if (Shape* shape = templateObj->maybeShape())
-        storePtr(ImmGCPtr(shape), Address(obj, ShapedObject::offsetOfShape()));
+    if (templateObj->is<ShapedObject>())
+        storePtr(ImmGCPtr(templateObj->maybeShape()), Address(obj, ShapedObject::offsetOfShape()));
 
     MOZ_ASSERT_IF(convertDoubleElements, templateObj->is<ArrayObject>());
 
     if (templateObj->isNative()) {
         NativeObject* ntemplate = &templateObj->as<NativeObject>();
         MOZ_ASSERT_IF(!ntemplate->denseElementsAreCopyOnWrite(), !ntemplate->hasDynamicElements());
 
         // If the object has dynamic slots, the slots member has already been
@@ -1279,17 +1279,18 @@ MacroAssembler::initGCThing(Register obj
         while (nbytes) {
             uintptr_t value = *(uintptr_t*)(memory + offset);
             storePtr(ImmWord(value),
                      Address(obj, InlineTypedObject::offsetOfDataStart() + offset));
             nbytes = (nbytes < sizeof(uintptr_t)) ? 0 : nbytes - sizeof(uintptr_t);
             offset += sizeof(uintptr_t);
         }
     } else if (templateObj->is<UnboxedPlainObject>()) {
-        storePtr(ImmWord(0), Address(obj, UnboxedPlainObject::offsetOfExpando()));
+        MOZ_ASSERT(!templateObj->as<UnboxedPlainObject>().maybeExpando());
+        storePtr(ImmPtr(nullptr), Address(obj, UnboxedPlainObject::offsetOfExpando()));
         if (initContents)
             initUnboxedObjectContents(obj, &templateObj->as<UnboxedPlainObject>());
     } else {
         MOZ_CRASH("Unknown object");
     }
 
 #ifdef JS_GC_TRACE
     RegisterSet regs = RegisterSet::Volatile();
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -123,16 +123,20 @@ class JSObject : public js::gc::Cell
         MOZ_ASSERT(!hasLazyGroup());
         return groupRaw();
     }
 
     js::ObjectGroup* groupRaw() const {
         return group_;
     }
 
+    void initGroup(js::ObjectGroup* group) {
+        group_.init(group);
+    }
+
     /*
      * Whether this is the only object which has its specified group. This
      * object will have its group constructed lazily as needed by analysis.
      */
     bool isSingleton() const {
         return group_->singleton();
     }
 
@@ -150,17 +154,16 @@ class JSObject : public js::gc::Cell
     inline js::Shape* maybeShape() const;
     inline js::Shape* ensureShape(JSContext* cx);
 
     // Set the initial slots and elements of an object. These pointers are only
     // valid for native objects, but during initialization are set for all
     // objects. For non-native objects, these must not be dynamically allocated
     // pointers which leak when the non-native object finishes initialization.
     inline void setInitialSlotsMaybeNonNative(js::HeapSlot* slots);
-    inline void setInitialElementsMaybeNonNative(js::HeapSlot* elements);
 
     enum GenerateShape {
         GENERATE_NONE,
         GENERATE_SHAPE
     };
 
     static bool setFlags(JSContext* cx, JS::HandleObject obj, js::BaseShape::Flag flags,
                          GenerateShape generateShape = GENERATE_NONE);
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -398,22 +398,16 @@ SetNewObjectMetadata(JSContext* cx, T* o
 } // namespace js
 
 inline void
 JSObject::setInitialSlotsMaybeNonNative(js::HeapSlot* slots)
 {
     static_cast<js::NativeObject*>(this)->slots_ = slots;
 }
 
-inline void
-JSObject::setInitialElementsMaybeNonNative(js::HeapSlot* elements)
-{
-    static_cast<js::NativeObject*>(this)->elements_ = elements;
-}
-
 inline js::GlobalObject&
 JSObject::global() const
 {
     /*
      * The global is read-barriered so that it is kept live by access through
      * the JSCompartment. When accessed through a JSObject, however, the global
      * will be already be kept live by the black JSObject's parent pointer, so
      * does not need to be read-barriered.
--- a/js/src/vm/ArrayObject-inl.h
+++ b/js/src/vm/ArrayObject-inl.h
@@ -32,39 +32,42 @@ ArrayObject::setLength(JSContext* cx, ui
     getElementsHeader()->length = length;
 }
 
 /* static */ inline ArrayObject*
 ArrayObject::createArrayInternal(JSContext* cx, gc::AllocKind kind, gc::InitialHeap heap,
                                  HandleShape shape, HandleObjectGroup group,
                                  AutoSetNewObjectMetadata&)
 {
-    // Create a new array and initialize everything except for its elements.
+    const js::Class* clasp = group->clasp();
     MOZ_ASSERT(shape && group);
-    MOZ_ASSERT(group->clasp() == shape->getObjectClass());
-    MOZ_ASSERT(group->clasp() == &ArrayObject::class_);
-    MOZ_ASSERT_IF(group->clasp()->hasFinalize(), heap == gc::TenuredHeap);
+    MOZ_ASSERT(clasp == shape->getObjectClass());
+    MOZ_ASSERT(clasp == &ArrayObject::class_);
+    MOZ_ASSERT_IF(clasp->hasFinalize(), heap == gc::TenuredHeap);
     MOZ_ASSERT_IF(group->hasUnanalyzedPreliminaryObjects(),
                   heap == js::gc::TenuredHeap);
-    MOZ_ASSERT(group->clasp()->shouldDelayMetadataBuilder());
 
     // Arrays can use their fixed slots to store elements, so can't have shapes
     // which allow named properties to be stored in the fixed slots.
     MOZ_ASSERT(shape->numFixedSlots() == 0);
 
-    size_t nDynamicSlots = dynamicSlotsCount(0, shape->slotSpan(), group->clasp());
-    JSObject* obj = Allocate<JSObject>(cx, kind, nDynamicSlots, heap, group->clasp());
+    size_t nDynamicSlots = dynamicSlotsCount(0, shape->slotSpan(), clasp);
+    JSObject* obj = js::Allocate<JSObject>(cx, kind, nDynamicSlots, heap, clasp);
     if (!obj)
         return nullptr;
 
-    static_cast<ArrayObject*>(obj)->shape_.init(shape);
-    static_cast<ArrayObject*>(obj)->group_.init(group);
+    ArrayObject* aobj = static_cast<ArrayObject*>(obj);
+    aobj->initGroup(group);
+    aobj->initShape(shape);
+    // NOTE: Slots are created and assigned internally by Allocate<JSObject>.
 
-    cx->compartment()->setObjectPendingMetadata(cx, obj);
-    return &obj->as<ArrayObject>();
+    MOZ_ASSERT(clasp->shouldDelayMetadataBuilder());
+    cx->compartment()->setObjectPendingMetadata(cx, aobj);
+
+    return aobj;
 }
 
 /* static */ inline ArrayObject*
 ArrayObject::finishCreateArray(ArrayObject* obj, HandleShape shape, AutoSetNewObjectMetadata& metadata)
 {
     size_t span = shape->slotSpan();
     if (span)
         obj->initializeSlotRange(0, span);
--- a/js/src/vm/Caches-inl.h
+++ b/js/src/vm/Caches-inl.h
@@ -55,17 +55,18 @@ NewObjectCache::newObjectFromHit(JSConte
     MOZ_ASSERT(!group->hasUnanalyzedPreliminaryObjects());
 
     if (group->shouldPreTenure())
         heap = gc::TenuredHeap;
 
     if (cx->runtime()->gc.upcomingZealousGC())
         return nullptr;
 
-    NativeObject* obj = static_cast<NativeObject*>(Allocate<JSObject, NoGC>(cx, entry->kind, 0,
+    NativeObject* obj = static_cast<NativeObject*>(Allocate<JSObject, NoGC>(cx, entry->kind,
+                                                                            /* nDynamicSlots = */ 0,
                                                                             heap, group->clasp()));
     if (!obj)
         return nullptr;
 
     copyCachedToObject(obj, templateObj, entry->kind);
 
     if (group->clasp()->shouldDelayMetadataBuilder())
         cx->compartment()->setObjectPendingMetadata(cx, obj);
--- a/js/src/vm/Caches.h
+++ b/js/src/vm/Caches.h
@@ -12,16 +12,17 @@
 #include "jsmath.h"
 #include "jsobj.h"
 #include "jsscript.h"
 
 #include "frontend/SourceNotes.h"
 #include "gc/Tracer.h"
 #include "js/RootingAPI.h"
 #include "js/UniquePtr.h"
+#include "vm/ArrayObject.h"
 #include "vm/NativeObject.h"
 
 namespace js {
 
 /*
  * GetSrcNote cache to avoid O(n^2) growth in finding a source note for a
  * given pc in a script. We use the script->code pointer to tag the cache,
  * instead of the script address itself, so that source notes are always found
@@ -195,16 +196,19 @@ class NewObjectCache
     }
 
     void fill(EntryIndex entry_, const Class* clasp, gc::Cell* key, gc::AllocKind kind,
               NativeObject* obj) {
         MOZ_ASSERT(unsigned(entry_) < mozilla::ArrayLength(entries));
         MOZ_ASSERT(entry_ == makeIndex(clasp, key, kind));
         Entry* entry = &entries[entry_];
 
+        MOZ_ASSERT(!obj->hasDynamicSlots());
+        MOZ_ASSERT(obj->hasEmptyElements() || obj->is<ArrayObject>());
+
         entry->clasp = clasp;
         entry->key = key;
         entry->kind = kind;
 
         entry->nbytes = gc::Arena::thingSize(kind);
         js_memcpy(&entry->templateObject, obj, entry->nbytes);
     }
 
--- a/js/src/vm/NativeObject-inl.h
+++ b/js/src/vm/NativeObject-inl.h
@@ -537,24 +537,23 @@ NativeObject::create(JSContext* cx, js::
 
     size_t nDynamicSlots = dynamicSlotsCount(shape->numFixedSlots(), shape->slotSpan(), clasp);
 
     JSObject* obj = js::Allocate<JSObject>(cx, kind, nDynamicSlots, heap, clasp);
     if (!obj)
         return cx->alreadyReportedOOM();
 
     NativeObject* nobj = static_cast<NativeObject*>(obj);
-    nobj->group_.init(group);
+    nobj->initGroup(group);
     nobj->initShape(shape);
-
-    // Note: slots are created and assigned internally by Allocate<JSObject>.
-    nobj->setInitialElementsMaybeNonNative(js::emptyObjectElements);
+    // NOTE: Slots are created and assigned internally by Allocate<JSObject>.
+    nobj->setEmptyElements();
 
     if (clasp->hasPrivate())
-        nobj->privateRef(shape->numFixedSlots()) = nullptr;
+        nobj->initPrivate(nullptr);
 
     if (size_t span = shape->slotSpan())
         nobj->initializeSlotRange(0, span);
 
     // JSFunction's fixed slots expect POD-style initialization.
     if (clasp->isJSFunction()) {
         MOZ_ASSERT(kind == js::gc::AllocKind::FUNCTION ||
                    kind == js::gc::AllocKind::FUNCTION_EXTENDED);
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@@ -1317,16 +1317,20 @@ class NativeObject : public ShapedObject
                       "slots will hold the ObjectElements header");
         return &fixedSlots()[2];
     }
 
 #ifdef DEBUG
     bool canHaveNonEmptyElements();
 #endif
 
+    void setEmptyElements() {
+        elements_ = emptyObjectElements;
+    }
+
     void setFixedElements(uint32_t numShifted = 0) {
         MOZ_ASSERT(canHaveNonEmptyElements());
         elements_ = fixedElements() + numShifted;
     }
 
     inline bool hasDynamicElements() const {
         /*
          * Note: for objects with zero fixed slots this could potentially give
--- a/js/src/vm/ProxyObject.cpp
+++ b/js/src/vm/ProxyObject.cpp
@@ -177,22 +177,22 @@ ProxyObject::create(JSContext* cx, const
             return cx->alreadyReportedOOM();
 
         comp->newProxyCache.add(group, shape);
     }
 
     gc::InitialHeap heap = GetInitialHeap(newKind, clasp);
     debugCheckNewObject(group, shape, allocKind, heap);
 
-    JSObject* obj = js::Allocate<JSObject>(cx, allocKind, /* numDynamicSlots = */ 0, heap, clasp);
+    JSObject* obj = js::Allocate<JSObject>(cx, allocKind, /* nDynamicSlots = */ 0, heap, clasp);
     if (!obj)
         return cx->alreadyReportedOOM();
 
     ProxyObject* pobj = static_cast<ProxyObject*>(obj);
-    pobj->group_.init(group);
+    pobj->initGroup(group);
     pobj->initShape(shape);
 
     MOZ_ASSERT(clasp->shouldDelayMetadataBuilder());
     cx->compartment()->setObjectPendingMetadata(cx, pobj);
 
     js::gc::TraceCreateObject(pobj);
 
     if (newKind == SingletonObject) {
--- a/js/src/vm/UnboxedObject.cpp
+++ b/js/src/vm/UnboxedObject.cpp
@@ -622,17 +622,17 @@ UnboxedObject::createInternal(JSContext*
 
     debugCheckNewObject(group, /* shape = */ nullptr, kind, heap);
 
     JSObject* obj = js::Allocate<JSObject>(cx, kind, /* nDynamicSlots = */ 0, heap, clasp);
     if (!obj)
         return cx->alreadyReportedOOM();
 
     UnboxedObject* uobj = static_cast<UnboxedObject*>(obj);
-    uobj->group_.init(group);
+    uobj->initGroup(group);
 
     MOZ_ASSERT(clasp->shouldDelayMetadataBuilder());
     cx->compartment()->setObjectPendingMetadata(cx, uobj);
 
     js::gc::TraceCreateObject(uobj);
 
     return uobj;
 }
@@ -644,44 +644,43 @@ UnboxedPlainObject::create(JSContext* cx
     AutoSetNewObjectMetadata metadata(cx);
 
     MOZ_ASSERT(group->clasp() == &class_);
     gc::AllocKind allocKind = group->unboxedLayout().getAllocKind();
     gc::InitialHeap heap = GetInitialHeap(newKind, &class_);
 
     MOZ_ASSERT(newKind != SingletonObject);
 
-    UnboxedObject* res_;
-    JS_TRY_VAR_OR_RETURN_NULL(cx, res_, createInternal(cx, allocKind, heap, group));
-    UnboxedPlainObject* res = &res_->as<UnboxedPlainObject>();
+    JSObject* obj;
+    JS_TRY_VAR_OR_RETURN_NULL(cx, obj, createInternal(cx, allocKind, heap, group));
 
-    // Overwrite the dummy shape which was written to the object's expando field.
-    res->initExpando();
+    UnboxedPlainObject* uobj = static_cast<UnboxedPlainObject*>(obj);
+    uobj->initExpando();
 
     // Initialize reference fields of the object. All fields in the object will
     // be overwritten shortly, but references need to be safe for the GC.
-    const int32_t* list = res->layout().traceList();
+    const int32_t* list = uobj->layout().traceList();
     if (list) {
-        uint8_t* data = res->data();
+        uint8_t* data = uobj->data();
         while (*list != -1) {
             GCPtrString* heap = reinterpret_cast<GCPtrString*>(data + *list);
             heap->init(cx->names().empty);
             list++;
         }
         list++;
         while (*list != -1) {
             GCPtrObject* heap = reinterpret_cast<GCPtrObject*>(data + *list);
             heap->init(nullptr);
             list++;
         }
         // Unboxed objects don't have Values to initialize.
         MOZ_ASSERT(*(list + 1) == -1);
     }
 
-    return res;
+    return uobj;
 }
 
 /* static */ JSObject*
 UnboxedPlainObject::createWithProperties(JSContext* cx, HandleObjectGroup group,
                                          NewObjectKind newKind, IdValuePair* properties)
 {
     MOZ_ASSERT(newKind == GenericObject || newKind == TenuredObject);