Bug 1322971 - Expose cells found via iteration. r=sfink, a=gchang
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 10 Feb 2017 10:22:38 +0000
changeset 312683 a0ead6ef09eb931ac334c715946cf798193c59ee
parent 312682 0a22becb23cde8ec54cb9a1c0c43382d80db63a3
child 312684 d3fede027d061293753fd940191061e973efb0f3
push id429
push userryanvm@gmail.com
push dateThu, 16 Feb 2017 15:23:16 +0000
treeherdermozilla-esr45@d3fede027d06 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink, gchang
bugs1322971
milestone45.7.1
Bug 1322971 - Expose cells found via iteration. r=sfink, a=gchang
js/public/GCAPI.h
js/src/jscompartment.cpp
js/src/jsgcinlines.h
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -561,18 +561,16 @@ UnmarkGrayGCThingRecursively(GCCellPtr t
 } /* namespace JS */
 
 namespace js {
 namespace gc {
 
 static MOZ_ALWAYS_INLINE void
 ExposeGCThingToActiveJS(JS::GCCellPtr thing)
 {
-    MOZ_ASSERT(thing.kind() != JS::TraceKind::Shape);
-
     /*
      * GC things residing in the nursery cannot be gray: they have no mark bits.
      * All live objects in the nursery are moved to tenured at the beginning of
      * each GC slice, so the gray marker never sees nursery things.
      */
     if (IsInsideNursery(thing.asCell()))
         return;
     JS::shadow::Runtime* rt = detail::GetGCThingRuntime(thing.unsafeAsUIntPtr());
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -952,25 +952,16 @@ AddLazyFunctionsForCompartment(JSContext
         // are about to be finalized. GC things referenced by objects that are
         // about to be finalized (e.g., in slots) may already be freed.
         if (gc::IsAboutToBeFinalizedUnbarriered(&fun) ||
             fun->compartment() != cx->compartment())
         {
             continue;
         }
 
-        // This creates a new reference to an object that an ongoing incremental
-        // GC may find to be unreachable. Treat as if we're reading a weak
-        // reference and trigger the read barrier.
-        if (cx->zone()->needsIncrementalBarrier())
-            fun->readBarrier(fun);
-
-        // TODO: The above checks should be rolled into the cell iterator (see
-        // bug 1322971).
-
         if (fun->isInterpretedLazy()) {
             LazyScript* lazy = fun->lazyScriptOrNull();
             if (lazy && lazy->sourceObject() && !lazy->hasUncompiledEnclosingScript()) {
                 if (!lazyFunctions.append(fun))
                     return false;
             }
         }
     }
--- a/js/src/jsgcinlines.h
+++ b/js/src/jsgcinlines.h
@@ -84,21 +84,29 @@ class ArenaIter
         if (!aheader) {
             aheader = unsweptHeader;
             unsweptHeader = sweptHeader;
             sweptHeader = nullptr;
         }
     }
 };
 
+enum CellIterNeedsBarrier : uint8_t
+{
+    CellIterDoesntNeedBarrier = 0,
+    CellIterMayNeedBarrier = 1
+};
+
 class ArenaCellIterImpl
 {
-    // These three are set in initUnsynchronized().
+    // These are set in initUnsynchronized().
     size_t firstThingOffset;
     size_t thingSize;
+    JS::TraceKind traceKind;
+    bool needsBarrier;
 #ifdef DEBUG
     bool isInited;
 #endif
 
     // These three are set in reset() (which is called by init()).
     FreeSpan span;
     uintptr_t thing;
     uintptr_t limit;
@@ -117,36 +125,40 @@ class ArenaCellIterImpl
             span = *span.nextSpan();
         }
     }
 
   public:
     ArenaCellIterImpl()
       : firstThingOffset(0)     // Squelch
       , thingSize(0)            //   warnings
+      , traceKind(JS::TraceKind::Null)
+      , needsBarrier(false)
       , limit(0)
     {
     }
 
-    void initUnsynchronized(ArenaHeader* aheader) {
+    void initUnsynchronized(ArenaHeader* aheader, CellIterNeedsBarrier mayNeedBarrier) {
         AllocKind kind = aheader->getAllocKind();
 #ifdef DEBUG
         isInited = true;
 #endif
         firstThingOffset = Arena::firstThingOffset(kind);
         thingSize = Arena::thingSize(kind);
+        traceKind = MapAllocToTraceKind(kind);
+        needsBarrier = mayNeedBarrier && !aheader->zone->runtimeFromMainThread()->isHeapCollecting();
         reset(aheader);
     }
 
-    void init(ArenaHeader* aheader) {
+    void init(ArenaHeader* aheader, CellIterNeedsBarrier mayNeedBarrier) {
 #ifdef DEBUG
         AllocKind kind = aheader->getAllocKind();
         MOZ_ASSERT(aheader->zone->arenas.isSynchronizedFreeList(kind));
 #endif
-        initUnsynchronized(aheader);
+        initUnsynchronized(aheader, mayNeedBarrier);
     }
 
     // Use this to move from an Arena of a particular kind to another Arena of
     // the same kind.
     void reset(ArenaHeader* aheader) {
         MOZ_ASSERT(isInited);
         span = aheader->getFirstFreeSpan();
         uintptr_t arenaAddr = aheader->arenaAddress();
@@ -156,17 +168,25 @@ class ArenaCellIterImpl
     }
 
     bool done() const {
         return thing == limit;
     }
 
     TenuredCell* getCell() const {
         MOZ_ASSERT(!done());
-        return reinterpret_cast<TenuredCell*>(thing);
+        TenuredCell* cell = reinterpret_cast<TenuredCell*>(thing);
+
+        // This can result in a a new reference being created to an object that
+        // an ongoing incremental GC may find to be unreachable, so we may need
+        // a barrier here.
+        if (needsBarrier)
+            ExposeGCThingToActiveJS(JS::GCCellPtr(cell, traceKind));
+
+        return cell;
     }
 
     template<typename T> T* get() const {
         MOZ_ASSERT(!done());
         return static_cast<T*>(getCell());
     }
 
     void next() {
@@ -181,41 +201,41 @@ template<>
 JSObject*
 ArenaCellIterImpl::get<JSObject>() const;
 
 class ArenaCellIterUnderGC : public ArenaCellIterImpl
 {
   public:
     explicit ArenaCellIterUnderGC(ArenaHeader* aheader) {
         MOZ_ASSERT(aheader->zone->runtimeFromAnyThread()->isHeapBusy());
-        init(aheader);
+        init(aheader, CellIterDoesntNeedBarrier);
     }
 };
 
 class ArenaCellIterUnderFinalize : public ArenaCellIterImpl
 {
   public:
     explicit ArenaCellIterUnderFinalize(ArenaHeader* aheader) {
-        initUnsynchronized(aheader);
+        initUnsynchronized(aheader, CellIterDoesntNeedBarrier);
     }
 };
 
 class ZoneCellIterImpl
 {
     ArenaIter arenaIter;
     ArenaCellIterImpl cellIter;
 
   protected:
     ZoneCellIterImpl() {}
 
     void init(JS::Zone* zone, AllocKind kind) {
         MOZ_ASSERT(zone->arenas.isSynchronizedFreeList(kind));
         arenaIter.init(zone, kind);
         if (!arenaIter.done())
-            cellIter.init(arenaIter.get());
+            cellIter.init(arenaIter.get(), CellIterMayNeedBarrier);
     }
 
   public:
     bool done() const {
         return arenaIter.done();
     }
 
     template<typename T> T* get() const {