Bug 1463939 - Make NativeIterator::obj_ private, give it a clearer name, and add public accessors to observe or modify it. r=jandem
authorJeff Walden <jwalden@mit.edu>
Wed, 23 May 2018 17:28:15 -0700
changeset 419788 8f1d10d34e9c5581abdf7088b26b4e7076baad6a
parent 419787 135af0c54dd7dda95f06eaf587b9d52fff5fa677
child 419789 ceccac3e93a1ccedffdfbf89becdd537910854cc
push id34049
push userbtara@mozilla.com
push dateFri, 25 May 2018 10:01:58 +0000
treeherdermozilla-central@bf4762f10b8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1463939
milestone62.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 1463939 - Make NativeIterator::obj_ private, give it a clearer name, and add public accessors to observe or modify it. r=jandem
js/src/jit/BaselineCacheIRCompiler.cpp
js/src/jit/IonCacheIRCompiler.cpp
js/src/proxy/CrossCompartmentWrapper.cpp
js/src/vm/Iteration.cpp
js/src/vm/Iteration.h
js/src/vm/JSCompartment-inl.h
--- a/js/src/jit/BaselineCacheIRCompiler.cpp
+++ b/js/src/jit/BaselineCacheIRCompiler.cpp
@@ -1978,26 +1978,26 @@ BaselineCacheIRCompiler::emitGuardAndGet
     // Load our PropertyIteratorObject* and its NativeIterator.
     masm.loadPtr(iterAddr, output);
     masm.loadObjPrivate(output, JSObject::ITER_CLASS_NFIXED_SLOTS, niScratch);
 
     // Ensure the |active| and |unreusable| bits are not set.
     masm.branchTest32(Assembler::NonZero, Address(niScratch, offsetof(NativeIterator, flags)),
                       Imm32(JSITER_ACTIVE|JSITER_UNREUSABLE), failure->label());
 
-    // Pre-write barrier for store to 'obj'.
-    Address iterObjAddr(niScratch, offsetof(NativeIterator, obj));
+    // Pre-write barrier for store to 'objectBeingIterated_'.
+    Address iterObjAddr(niScratch, NativeIterator::offsetOfObjectBeingIterated());
     EmitPreBarrier(masm, iterObjAddr, MIRType::Object);
 
     // Mark iterator as active.
     Address iterFlagsAddr(niScratch, offsetof(NativeIterator, flags));
     masm.storePtr(obj, iterObjAddr);
     masm.or32(Imm32(JSITER_ACTIVE), iterFlagsAddr);
 
-    // Post-write barrier for stores to 'obj'.
+    // Post-write barrier for stores to 'objectBeingIterated_'.
     emitPostBarrierSlot(output, TypedOrValueRegister(MIRType::Object, AnyRegister(obj)), scratch1);
 
     // Chain onto the active iterator stack. Note that Baseline CacheIR stub
     // code is shared across compartments within a Zone, so we can't bake in
     // compartment->enumerators here.
     masm.loadPtr(enumeratorsAddr, scratch1);
     masm.loadPtr(Address(scratch1, 0), scratch1);
     emitRegisterEnumerator(scratch1, niScratch, scratch2);
--- a/js/src/jit/IonCacheIRCompiler.cpp
+++ b/js/src/jit/IonCacheIRCompiler.cpp
@@ -2307,26 +2307,26 @@ IonCacheIRCompiler::emitGuardAndGetItera
     // Load our PropertyIteratorObject* and its NativeIterator.
     masm.movePtr(ImmGCPtr(iterobj), output);
     masm.loadObjPrivate(output, JSObject::ITER_CLASS_NFIXED_SLOTS, niScratch);
 
     // Ensure the |active| and |unreusable| bits are not set.
     masm.branchTest32(Assembler::NonZero, Address(niScratch, offsetof(NativeIterator, flags)),
                       Imm32(JSITER_ACTIVE|JSITER_UNREUSABLE), failure->label());
 
-    // Pre-write barrier for store to 'obj'.
-    Address iterObjAddr(niScratch, offsetof(NativeIterator, obj));
+    // Pre-write barrier for store to 'objectBeingIterated_'.
+    Address iterObjAddr(niScratch, NativeIterator::offsetOfObjectBeingIterated());
     EmitPreBarrier(masm, iterObjAddr, MIRType::Object);
 
     // Mark iterator as active.
     Address iterFlagsAddr(niScratch, offsetof(NativeIterator, flags));
     masm.storePtr(obj, iterObjAddr);
     masm.or32(Imm32(JSITER_ACTIVE), iterFlagsAddr);
 
-    // Post-write barrier for stores to 'obj'.
+    // Post-write barrier for stores to 'objectBeingIterated_'.
     emitPostBarrierSlot(output, TypedOrValueRegister(MIRType::Object, AnyRegister(obj)), scratch1);
 
     // Chain onto the active iterator stack.
     masm.loadPtr(AbsoluteAddress(enumerators), scratch1);
     emitRegisterEnumerator(scratch1, niScratch, scratch2);
 
     return true;
 }
--- a/js/src/proxy/CrossCompartmentWrapper.cpp
+++ b/js/src/proxy/CrossCompartmentWrapper.cpp
@@ -278,17 +278,17 @@ struct AutoCloseIterator
 };
 
 static JSObject*
 Reify(JSContext* cx, JSCompartment* origin, HandleObject objp)
 {
     Rooted<PropertyIteratorObject*> iterObj(cx, &objp->as<PropertyIteratorObject>());
     NativeIterator* ni = iterObj->getNativeIterator();
 
-    RootedObject obj(cx, ni->obj);
+    RootedObject obj(cx, &ni->objectBeingIterated());
     {
         AutoCloseIterator close(cx, iterObj);
 
         /* Wrap the iteratee. */
         if (!origin->wrap(cx, &obj))
             return nullptr;
 
         /*
--- a/js/src/vm/Iteration.cpp
+++ b/js/src/vm/Iteration.cpp
@@ -52,17 +52,17 @@ using mozilla::PodEqual;
 
 typedef Rooted<PropertyIteratorObject*> RootedPropertyIteratorObject;
 
 static const gc::AllocKind ITERATOR_FINALIZE_KIND = gc::AllocKind::OBJECT2_BACKGROUND;
 
 void
 NativeIterator::trace(JSTracer* trc)
 {
-    TraceNullableEdge(trc, &obj, "obj");
+    TraceNullableEdge(trc, &objectBeingIterated_, "objectBeingIterated_");
 
     // The SuppressDeletedPropertyHelper loop can GC, so make sure that if the
     // GC removes any elements from the list, it won't remove this one.
     if (iterObj_)
         TraceManuallyBarrieredEdge(trc, &iterObj_, "iterObj");
 
     std::for_each(guardsBegin(), guardsEnd(),
                   [trc](HeapReceiverGuard& guard) {
@@ -653,17 +653,17 @@ NativeIterator::allocateSentinel(JSConte
  *
  * This definition is a bit tricky: some parts of initializing are fallible, so
  * as we initialize, we must carefully keep this in GC-safe state (see
  * NativeIterator::trace).
  */
 NativeIterator::NativeIterator(JSContext* cx, Handle<PropertyIteratorObject*> propIter,
                                Handle<JSObject*> objBeingIterated, const AutoIdVector& props,
                                uint32_t numGuards, uint32_t guardKey, bool* hadError)
-  : obj(objBeingIterated),
+  : objectBeingIterated_(objBeingIterated),
     iterObj_(propIter),
     // NativeIterator initially acts (before full initialization) as if it
     // contains no guards...
     guardsEnd_(guardsBegin()),
     // ...and no properties.
     propertyCursor_(reinterpret_cast<GCPtrFlatString*>(guardsBegin() + numGuards)),
     propertiesEnd_(propertyCursor_),
     guard_key(guardKey),
@@ -768,24 +768,16 @@ IteratorHashPolicy::match(PropertyIterat
     NativeIterator* ni = obj->getNativeIterator();
     if (ni->guard_key != lookup.key || ni->guardCount() != lookup.numGuards)
         return false;
 
     return PodEqual(reinterpret_cast<ReceiverGuard*>(ni->guardsBegin()), lookup.guards,
                     ni->guardCount());
 }
 
-static inline void
-UpdateNativeIterator(NativeIterator* ni, JSObject* obj)
-{
-    // Update the object for which the native iterator is associated, so
-    // SuppressDeletedPropertyHelper will recognize the iterator as a match.
-    ni->obj = obj;
-}
-
 static inline bool
 CanCompareIterableObjectToCache(JSObject* obj)
 {
     if (obj->isNative())
         return obj->as<NativeObject>().hasEmptyElements();
     if (obj->is<UnboxedPlainObject>()) {
         if (UnboxedExpandoObject* expando = obj->as<UnboxedPlainObject>().maybeExpando())
             return expando->hasEmptyElements();
@@ -893,17 +885,17 @@ StoreInIteratorCache(JSContext* cx, JSOb
 }
 
 JSObject*
 js::GetIterator(JSContext* cx, HandleObject obj)
 {
     uint32_t numGuards = 0;
     if (PropertyIteratorObject* iterobj = LookupInIteratorCache(cx, obj, &numGuards)) {
         NativeIterator* ni = iterobj->getNativeIterator();
-        UpdateNativeIterator(ni, obj);
+        ni->changeObjectBeingIterated(*obj);
         RegisterEnumerator(ObjectRealm::get(obj), ni);
         return iterobj;
     }
 
     if (numGuards > 0 && !CanStoreInIteratorCache(obj))
         numGuards = 0;
 
     MOZ_ASSERT(!obj->is<PropertyIteratorObject>());
@@ -1248,17 +1240,17 @@ js::UnwindIteratorForUncatchableExceptio
         ni->unlink();
     }
 }
 
 static bool
 SuppressDeletedProperty(JSContext* cx, NativeIterator* ni, HandleObject obj,
                         Handle<JSFlatString*> str)
 {
-    if (ni->obj != obj)
+    if (ni->objectBeingIterated() != *obj)
         return true;
 
     // Optimization for the following common case:
     //
     //    for (var p in o) {
     //        delete o[p];
     //    }
     //
--- a/js/src/vm/Iteration.h
+++ b/js/src/vm/Iteration.h
@@ -27,21 +27,20 @@
 #define JSITER_UNREUSABLE   0x2000
 
 namespace js {
 
 class PropertyIteratorObject;
 
 struct NativeIterator
 {
-  public:
-    // Object being iterated.
-    GCPtrObject obj = {};
+  private:
+    // Object being iterated.  Non-null except in NativeIterator sentinels.
+    GCPtrObject objectBeingIterated_ = {};
 
-  private:
     // Internal iterator object.
     JSObject* iterObj_ = nullptr;
 
     // The end of HeapReceiverGuards that appear directly after |this|, as part
     // of an overall allocation that stores |*this|, receiver guards, and
     // iterated strings.  Once this has been fully initialized, it also equals
     // the start of iterated strings.
     HeapReceiverGuard* guardsEnd_; // initialized by constructor
@@ -83,16 +82,24 @@ struct NativeIterator
      */
     NativeIterator(JSContext* cx, Handle<PropertyIteratorObject*> propIter,
                    Handle<JSObject*> objBeingIterated, const AutoIdVector& props,
                    uint32_t numGuards, uint32_t guardKey, bool* hadError);
 
     /** Initialize a |JSCompartment::enumerators| sentinel. */
     NativeIterator();
 
+    JSObject& objectBeingIterated() const {
+        return *objectBeingIterated_;
+    }
+
+    void changeObjectBeingIterated(JSObject& obj) {
+        objectBeingIterated_ = &obj;
+    }
+
     HeapReceiverGuard* guardsBegin() const {
         static_assert(alignof(HeapReceiverGuard) <= alignof(NativeIterator),
                       "NativeIterator must be aligned to begin storing "
                       "HeapReceiverGuards immediately after it with no "
                       "required padding");
         const NativeIterator* immediatelyAfter = this + 1;
         auto* afterNonConst = const_cast<NativeIterator*>(immediatelyAfter);
         return reinterpret_cast<HeapReceiverGuard*>(afterNonConst);
@@ -138,23 +145,16 @@ struct NativeIterator
         MOZ_ASSERT(propertyCursor_ < propertiesEnd());
         return propertyCursor_;
     }
 
     NativeIterator* next() {
         return next_;
     }
 
-    static inline size_t offsetOfNext() {
-        return offsetof(NativeIterator, next_);
-    }
-    static inline size_t offsetOfPrev() {
-        return offsetof(NativeIterator, prev_);
-    }
-
     void incCursor() {
         propertyCursor_++;
     }
     void link(NativeIterator* other) {
         /* A NativeIterator cannot appear in the enumerator list twice. */
         MOZ_ASSERT(!next_ && !prev_);
 
         this->next_ = other;
@@ -168,27 +168,39 @@ struct NativeIterator
         next_ = nullptr;
         prev_ = nullptr;
     }
 
     static NativeIterator* allocateSentinel(JSContext* maybecx);
 
     void trace(JSTracer* trc);
 
+    static constexpr size_t offsetOfObjectBeingIterated() {
+        return offsetof(NativeIterator, objectBeingIterated_);
+    }
+
     static constexpr size_t offsetOfGuardsEnd() {
         return offsetof(NativeIterator, guardsEnd_);
     }
 
     static constexpr size_t offsetOfPropertyCursor() {
         return offsetof(NativeIterator, propertyCursor_);
     }
 
     static constexpr size_t offsetOfPropertiesEnd() {
         return offsetof(NativeIterator, propertiesEnd_);
     }
+
+    static constexpr size_t offsetOfNext() {
+        return offsetof(NativeIterator, next_);
+    }
+
+    static constexpr size_t offsetOfPrev() {
+        return offsetof(NativeIterator, prev_);
+    }
 };
 
 class PropertyIteratorObject : public NativeObject
 {
     static const ClassOps classOps_;
 
   public:
     static const Class class_;
--- a/js/src/vm/JSCompartment-inl.h
+++ b/js/src/vm/JSCompartment-inl.h
@@ -172,14 +172,14 @@ js::ObjectRealm::objectMaybeInIteration(
 
     // If the list is empty we're not iterating any objects.
     js::NativeIterator* next = enumerators->next();
     if (enumerators == next)
         return false;
 
     // If the list contains a single object, check if it's |obj|.
     if (next->next() == enumerators)
-        return next->obj == obj;
+        return &next->objectBeingIterated() == obj;
 
     return true;
 }
 
 #endif /* vm_JSCompartment_inl_h */