Backed out changeset 622041bc3409 (bug 1322971) for bustage.
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 15 Feb 2017 21:05:50 -0500
changeset 378478 17ae86cbd8e13e594205d553384e06bd073ea826
parent 378477 b3af8fd8abe87f41452162724c1416ba3a82a460
child 378479 d956e48d28dfd1b6cc180b92b1bf170f0438569e
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1322971
milestone53.0a2
backs out622041bc3409ca8399ead2ea92ccf1825c22b51e
Backed out changeset 622041bc3409 (bug 1322971) for bustage.
js/public/GCAPI.h
js/src/jscompartment.cpp
js/src/jsgc.cpp
js/src/jsgcinlines.h
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -626,16 +626,18 @@ 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;
 
     // There's nothing to do for permanent GC things that might be owned by
     // another runtime.
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -1044,16 +1044,25 @@ 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/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3544,18 +3544,18 @@ ArenaLists::checkEmptyArenaList(AllocKin
 #ifdef DEBUG
     if (!arenaLists[kind].isEmpty()) {
         size_t max_cells = 20;
         char *env = getenv("JS_GC_MAX_LIVE_CELLS");
         if (env && *env)
             max_cells = atol(env);
         for (Arena* current = arenaLists[kind].head(); current; current = current->next) {
             for (ArenaCellIterUnderGC i(current); !i.done(); i.next()) {
-                TenuredCell* t = i.getCell();
-                MOZ_ASSERT(t->isMarked(), "unmarked cells should have been finalized");
+                Cell* t = i.get<Cell>();
+                MOZ_ASSERT(t->asTenured().isMarked(), "unmarked cells should have been finalized");
                 if (++num_live <= max_cells) {
                     fprintf(stderr, "ERROR: GC found live Cell %p of kind %s at shutdown\n",
                             t, AllocKindToAscii(kind));
                 }
             }
         }
         fprintf(stderr, "ERROR: GC found %" PRIuSIZE " live Cells at shutdown\n", num_live);
     }
--- a/js/src/jsgcinlines.h
+++ b/js/src/jsgcinlines.h
@@ -97,31 +97,23 @@ class ArenaIter
         if (!arena) {
             arena = unsweptArena;
             unsweptArena = sweptArena;
             sweptArena = nullptr;
         }
     }
 };
 
-enum CellIterNeedsBarrier : uint8_t
-{
-    CellIterDoesntNeedBarrier = 0,
-    CellIterMayNeedBarrier = 1
-};
-
 class ArenaCellIterImpl
 {
     size_t firstThingOffset;
     size_t thingSize;
     Arena* arenaAddr;
     FreeSpan span;
     uint_fast16_t thing;
-    JS::TraceKind traceKind;
-    bool needsBarrier;
     mozilla::DebugOnly<bool> initialized;
 
     // Upon entry, |thing| points to any thing (free or used) and finds the
     // first used thing, which may be |thing|.
     void moveForwardIfFree() {
         MOZ_ASSERT(!done());
         MOZ_ASSERT(thing);
         // Note: if |span| is empty, this test will fail, which is what we want
@@ -131,42 +123,27 @@ class ArenaCellIterImpl
         if (thing == span.first) {
             thing = span.last + thingSize;
             span = *span.nextSpan(arenaAddr);
         }
     }
 
   public:
     ArenaCellIterImpl()
-      : firstThingOffset(0),
-        thingSize(0),
-        arenaAddr(nullptr),
-        thing(0),
-        traceKind(JS::TraceKind::Null),
-        needsBarrier(false),
-        initialized(false)
-    {}
+      : firstThingOffset(0), thingSize(0), arenaAddr(nullptr), thing(0), initialized(false) {}
 
-    explicit ArenaCellIterImpl(Arena* arena, CellIterNeedsBarrier mayNeedBarrier)
-      : initialized(false)
-    {
-        init(arena, mayNeedBarrier);
-    }
+    explicit ArenaCellIterImpl(Arena* arena) : initialized(false) { init(arena); }
 
-    void init(Arena* arena, CellIterNeedsBarrier mayNeedBarrier) {
+    void init(Arena* arena) {
         MOZ_ASSERT(!initialized);
         MOZ_ASSERT(arena);
-        MOZ_ASSERT_IF(!mayNeedBarrier,
-                      CurrentThreadIsPerformingGC() || CurrentThreadIsGCSweeping());
         initialized = true;
         AllocKind kind = arena->getAllocKind();
         firstThingOffset = Arena::firstThingOffset(kind);
         thingSize = Arena::thingSize(kind);
-        traceKind = MapAllocToTraceKind(kind);
-        needsBarrier = mayNeedBarrier && !JS::CurrentThreadIsHeapCollecting();
         reset(arena);
     }
 
     // Use this to move from an Arena of a particular kind to another Arena of
     // the same kind.
     void reset(Arena* arena) {
         MOZ_ASSERT(initialized);
         MOZ_ASSERT(arena);
@@ -179,30 +156,21 @@ class ArenaCellIterImpl
     bool done() const {
         MOZ_ASSERT(initialized);
         MOZ_ASSERT(thing <= ArenaSize);
         return thing == ArenaSize;
     }
 
     TenuredCell* getCell() const {
         MOZ_ASSERT(!done());
-        TenuredCell* cell = reinterpret_cast<TenuredCell*>(uintptr_t(arenaAddr) + 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;
+        return reinterpret_cast<TenuredCell*>(uintptr_t(arenaAddr) + thing);
     }
 
     template<typename T> T* get() const {
         MOZ_ASSERT(!done());
-        MOZ_ASSERT(JS::MapTypeToTraceKind<T>::kind == traceKind);
         return static_cast<T*>(getCell());
     }
 
     void next() {
         MOZ_ASSERT(!done());
         thing += thingSize;
         if (thing < ArenaSize)
             moveForwardIfFree();
@@ -212,37 +180,37 @@ class ArenaCellIterImpl
 template<>
 JSObject*
 ArenaCellIterImpl::get<JSObject>() const;
 
 class ArenaCellIter : public ArenaCellIterImpl
 {
   public:
     explicit ArenaCellIter(Arena* arena)
-      : ArenaCellIterImpl(arena, CellIterMayNeedBarrier)
+      : ArenaCellIterImpl(arena)
     {
         MOZ_ASSERT(arena->zone->runtimeFromMainThread()->isHeapTracing());
     }
 };
 
 class ArenaCellIterUnderGC : public ArenaCellIterImpl
 {
   public:
     explicit ArenaCellIterUnderGC(Arena* arena)
-      : ArenaCellIterImpl(arena, CellIterDoesntNeedBarrier)
+      : ArenaCellIterImpl(arena)
     {
         MOZ_ASSERT(CurrentThreadIsPerformingGC());
     }
 };
 
 class ArenaCellIterUnderFinalize : public ArenaCellIterImpl
 {
   public:
     explicit ArenaCellIterUnderFinalize(Arena* arena)
-      : ArenaCellIterImpl(arena, CellIterDoesntNeedBarrier)
+      : ArenaCellIterImpl(arena)
     {
         MOZ_ASSERT(CurrentThreadIsGCSweeping());
     }
 };
 
 template <typename T>
 class ZoneCellIter;
 
@@ -275,17 +243,17 @@ class ZoneCellIter<TenuredCell> {
         // We have a single-threaded runtime, so there's no need to protect
         // against other threads iterating or allocating. However, we do have
         // background finalization; we may have to wait for this to finish if
         // it's currently active.
         if (IsBackgroundFinalized(kind) && zone->arenas.needBackgroundFinalizeWait(kind))
             rt->gc.waitBackgroundSweepEnd();
         arenaIter.init(zone, kind);
         if (!arenaIter.done())
-            cellIter.init(arenaIter.get(), CellIterMayNeedBarrier);
+            cellIter.init(arenaIter.get());
     }
 
   public:
     ZoneCellIter(JS::Zone* zone, AllocKind kind) {
         // If we are iterating a nursery-allocated kind then we need to
         // evict first so that we can see all things.
         if (IsNurseryAllocable(kind)) {
             JSRuntime* rt = zone->runtimeFromMainThread();