Bug 1394831 part 11 - Split getChildProperty in data vs accessor versions. r=bhackett
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 08 Nov 2017 15:58:31 +0100
changeset 444011 b44bda5764ca8615351259095b33a96f4c07a384
parent 444010 9d8e2a8c4c03d70aa8aa841d8519a7e31615c5ef
child 444012 aba68ef3a080d52fd790c68e974ff85653577dc1
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1394831
milestone58.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 1394831 part 11 - Split getChildProperty in data vs accessor versions. r=bhackett
js/src/vm/NativeObject.h
js/src/vm/Shape.cpp
--- a/js/src/vm/NativeObject.h
+++ b/js/src/vm/NativeObject.h
@@ -842,19 +842,22 @@ class NativeObject : public ShapedObject
      * FIXME: bug 593129 -- slot allocation should be done by object methods
      * after calling object-parameter-free shape methods, avoiding coupling
      * logic across the object vs. shape module wall.
      */
     static bool allocDictionarySlot(JSContext* cx, HandleNativeObject obj, uint32_t* slotp);
     void freeSlot(JSContext* cx, uint32_t slot);
 
   private:
-    static MOZ_ALWAYS_INLINE Shape* getChildProperty(JSContext* cx, HandleNativeObject obj,
-                                                     HandleShape parent,
-                                                     MutableHandle<StackShape> child);
+    static MOZ_ALWAYS_INLINE Shape* getChildDataProperty(JSContext* cx, HandleNativeObject obj,
+                                                         HandleShape parent,
+                                                         MutableHandle<StackShape> child);
+    static MOZ_ALWAYS_INLINE Shape* getChildAccessorProperty(JSContext* cx, HandleNativeObject obj,
+                                                             HandleShape parent,
+                                                             MutableHandle<StackShape> child);
 
   public:
     /* Add a property whose id is not yet in this scope. */
     static MOZ_ALWAYS_INLINE Shape* addDataProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
                                                     uint32_t slot, unsigned attrs);
 
     static MOZ_ALWAYS_INLINE Shape* addAccessorProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
                                                         JSGetterOp getter, JSSetterOp setter,
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -287,66 +287,59 @@ Shape::replaceLastProperty(JSContext* cx
 }
 
 /*
  * Get or create a property-tree or dictionary child property of |parent|,
  * which must be lastProperty() if inDictionaryMode(), else parent must be
  * one of lastProperty() or lastProperty()->parent.
  */
 /* static */ MOZ_ALWAYS_INLINE Shape*
-NativeObject::getChildProperty(JSContext* cx,
-                               HandleNativeObject obj, HandleShape parent,
-                               MutableHandle<StackShape> child)
+NativeObject::getChildDataProperty(JSContext* cx,
+                                   HandleNativeObject obj, HandleShape parent,
+                                   MutableHandle<StackShape> child)
 {
-    /*
-     * Shared properties have no slot, but slot_ will reflect that of parent.
-     * Unshared properties allocate a slot here but may lose it due to a
-     * JS_ClearScope call.
-     */
-    if (!child.isDataProperty()) {
-        child.setSlot(parent->maybeSlot());
+    MOZ_ASSERT(child.isDataProperty());
+
+    if (child.hasMissingSlot()) {
+        uint32_t slot;
+        if (obj->inDictionaryMode()) {
+            if (!allocDictionarySlot(cx, obj, &slot))
+                return nullptr;
+        } else {
+            slot = obj->slotSpan();
+            MOZ_ASSERT(slot >= JSSLOT_FREE(obj->getClass()));
+            // Objects with many properties are converted to dictionary
+            // mode, so we can't overflow SHAPE_MAXIMUM_SLOT here.
+            MOZ_ASSERT(slot < JSSLOT_FREE(obj->getClass()) + PropertyTree::MAX_HEIGHT);
+            MOZ_ASSERT(slot < SHAPE_MAXIMUM_SLOT);
+        }
+        child.setSlot(slot);
     } else {
-        if (child.hasMissingSlot()) {
-            uint32_t slot;
-            if (obj->inDictionaryMode()) {
-                if (!allocDictionarySlot(cx, obj, &slot))
-                    return nullptr;
-            } else {
-                slot = obj->slotSpan();
-                MOZ_ASSERT(slot >= JSSLOT_FREE(obj->getClass()));
-                // Objects with many properties are converted to dictionary
-                // mode, so we can't overflow SHAPE_MAXIMUM_SLOT here.
-                MOZ_ASSERT(slot < JSSLOT_FREE(obj->getClass()) + PropertyTree::MAX_HEIGHT);
-                MOZ_ASSERT(slot < SHAPE_MAXIMUM_SLOT);
-            }
-            child.setSlot(slot);
-        } else {
-            /*
-             * Slots can only be allocated out of order on objects in
-             * dictionary mode.  Otherwise the child's slot must be after the
-             * parent's slot (if it has one), because slot number determines
-             * slot span for objects with that shape.  Usually child slot
-             * *immediately* follows parent slot, but there may be a slot gap
-             * when the object uses some -- but not all -- of its reserved
-             * slots to store properties.
-             */
-            MOZ_ASSERT(obj->inDictionaryMode() ||
-                       parent->hasMissingSlot() ||
-                       child.slot() == parent->maybeSlot() + 1 ||
-                       (parent->maybeSlot() + 1 < JSSLOT_FREE(obj->getClass()) &&
-                        child.slot() == JSSLOT_FREE(obj->getClass())));
-        }
+        /*
+         * Slots can only be allocated out of order on objects in
+         * dictionary mode.  Otherwise the child's slot must be after the
+         * parent's slot (if it has one), because slot number determines
+         * slot span for objects with that shape.  Usually child slot
+         * *immediately* follows parent slot, but there may be a slot gap
+         * when the object uses some -- but not all -- of its reserved
+         * slots to store properties.
+         */
+        MOZ_ASSERT(obj->inDictionaryMode() ||
+                   parent->hasMissingSlot() ||
+                   child.slot() == parent->maybeSlot() + 1 ||
+                   (parent->maybeSlot() + 1 < JSSLOT_FREE(obj->getClass()) &&
+                    child.slot() == JSSLOT_FREE(obj->getClass())));
     }
 
     if (obj->inDictionaryMode()) {
         MOZ_ASSERT(parent == obj->lastProperty());
-        Shape* shape = child.isAccessorShape() ? Allocate<AccessorShape>(cx) : Allocate<Shape>(cx);
+        Shape* shape = Allocate<Shape>(cx);
         if (!shape)
             return nullptr;
-        if (child.isDataProperty() && child.slot() >= obj->lastProperty()->base()->slotSpan()) {
+        if (child.slot() >= obj->lastProperty()->base()->slotSpan()) {
             if (!obj->setSlotSpan(cx, child.slot() + 1)) {
                 new (shape) Shape(obj->lastProperty()->base()->unowned(), 0);
                 return nullptr;
             }
         }
         shape->initDictionaryShape(child, obj->numFixedSlots(), &obj->shape_);
         return shape;
     }
@@ -358,16 +351,47 @@ NativeObject::getChildProperty(JSContext
     MOZ_ASSERT(shape->parent == parent);
     MOZ_ASSERT_IF(parent != obj->lastProperty(), parent == obj->lastProperty()->parent);
 
     if (!obj->setLastProperty(cx, shape))
         return nullptr;
     return shape;
 }
 
+/* static */ MOZ_ALWAYS_INLINE Shape*
+NativeObject::getChildAccessorProperty(JSContext* cx,
+                                       HandleNativeObject obj, HandleShape parent,
+                                       MutableHandle<StackShape> child)
+{
+    MOZ_ASSERT(!child.isDataProperty());
+
+    // Accessor properties have no slot, but slot_ will reflect that of parent.
+    child.setSlot(parent->maybeSlot());
+
+    if (obj->inDictionaryMode()) {
+        MOZ_ASSERT(parent == obj->lastProperty());
+        Shape* shape = Allocate<AccessorShape>(cx);
+        if (!shape)
+            return nullptr;
+        shape->initDictionaryShape(child, obj->numFixedSlots(), &obj->shape_);
+        return shape;
+    }
+
+    Shape* shape = cx->zone()->propertyTree().inlinedGetChild(cx, parent, child);
+    if (!shape)
+        return nullptr;
+
+    MOZ_ASSERT(shape->parent == parent);
+    MOZ_ASSERT_IF(parent != obj->lastProperty(), parent == obj->lastProperty()->parent);
+
+    if (!obj->setLastProperty(cx, shape))
+        return nullptr;
+    return shape;
+}
+
 /* static */ bool
 js::NativeObject::toDictionaryMode(JSContext* cx, HandleNativeObject obj)
 {
     MOZ_ASSERT(!obj->inDictionaryMode());
     MOZ_ASSERT(cx->isInsideCurrentCompartment(obj));
 
     uint32_t span = obj->slotSpan();
 
@@ -516,17 +540,17 @@ NativeObject::addAccessorPropertyInterna
     {
         RootedShape last(cx, obj->lastProperty());
         Rooted<UnownedBaseShape*> nbase(cx, GetBaseShapeForNewShape(cx, last, id));
         if (!nbase)
             return nullptr;
 
         Rooted<StackShape> child(cx, StackShape(nbase, id, SHAPE_INVALID_SLOT, attrs, 0));
         child.updateGetterSetter(getter, setter);
-        shape = getChildProperty(cx, obj, last, &child);
+        shape = getChildAccessorProperty(cx, obj, last, &child);
         if (!shape)
             return nullptr;
     }
 
     MOZ_ASSERT(shape == obj->lastProperty());
 
     if (table) {
         /* Store the tree node pointer in the table entry for id. */
@@ -583,17 +607,17 @@ NativeObject::addDataPropertyInternal(JS
     RootedShape shape(cx);
     {
         RootedShape last(cx, obj->lastProperty());
         Rooted<UnownedBaseShape*> nbase(cx, GetBaseShapeForNewShape(cx, last, id));
         if (!nbase)
             return nullptr;
 
         Rooted<StackShape> child(cx, StackShape(nbase, id, slot, attrs, 0));
-        shape = getChildProperty(cx, obj, last, &child);
+        shape = getChildDataProperty(cx, obj, last, &child);
         if (!shape)
             return nullptr;
     }
 
     MOZ_ASSERT(shape == obj->lastProperty());
 
     if (table) {
         /* Store the tree node pointer in the table entry for id. */
@@ -900,17 +924,17 @@ NativeObject::putDataProperty(JSContext*
         if (!nbase)
             return nullptr;
 
         MOZ_ASSERT(shape == obj->lastProperty());
 
         /* Find or create a property tree node labeled by our arguments. */
         Rooted<StackShape> child(cx, StackShape(nbase, id, slot, attrs, 0));
         RootedShape parent(cx, shape->parent);
-        shape = getChildProperty(cx, obj, parent, &child);
+        shape = getChildDataProperty(cx, obj, parent, &child);
         if (!shape)
             return nullptr;
     }
 
     MOZ_ASSERT(shape->isDataProperty());
     return shape;
 }
 
@@ -1031,17 +1055,17 @@ NativeObject::putAccessorProperty(JSCont
             return nullptr;
 
         MOZ_ASSERT(shape == obj->lastProperty());
 
         /* Find or create a property tree node labeled by our arguments. */
         Rooted<StackShape> child(cx, StackShape(nbase, id, SHAPE_INVALID_SLOT, attrs, 0));
         child.updateGetterSetter(getter, setter);
         RootedShape parent(cx, shape->parent);
-        shape = getChildProperty(cx, obj, parent, &child);
+        shape = getChildAccessorProperty(cx, obj, parent, &child);
         if (!shape)
             return nullptr;
     }
 
     /*
      * Can't fail now, so free the previous incarnation's slot. But we do not
      * need to free oldSlot (and must not, as trying to will botch an assertion
      * in JSObject::freeSlot) if the new last property (shape here) has a