bug 711623 - rt->gcNumFreeArenas is not updated properly. r=terrence
authorIgor Bukanov <igor@mir2.org>
Mon, 19 Dec 2011 23:07:24 +0100
changeset 84716 42b04bce9deb9a135e086061c489bf7ef40ff9ec
parent 84715 8aeec2db24d2ccef43e1654e098d5227cd4b1231
child 84717 d9ec0c88fb8686d6c007ac4ab24439a013da44d9
push id519
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 00:38:35 +0000
treeherdermozilla-beta@788ea1ef610b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs711623
milestone11.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 711623 - rt->gcNumFreeArenas is not updated properly. r=terrence
js/src/jsapi.cpp
js/src/jscntxt.h
js/src/jsgc.cpp
js/src/jsgc.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -646,17 +646,17 @@ JSRuntime::JSRuntime()
     gcUserAvailableChunkListHead(NULL),
     gcKeepAtoms(0),
     gcBytes(0),
     gcTriggerBytes(0),
     gcLastBytes(0),
     gcMaxBytes(0),
     gcMaxMallocBytes(0),
     gcEmptyArenaPoolLifespan(0),
-    gcNumFreeArenas(0),
+    gcNumArenasFreeCommitted(0),
     gcNumber(0),
     gcIncrementalTracer(NULL),
     gcVerifyData(NULL),
     gcChunkAllocationSinceLastGC(false),
     gcNextFullGCTime(0),
     gcJitReleaseTime(0),
     gcMode(JSGC_MODE_GLOBAL),
     gcIsNeeded(0),
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -419,18 +419,23 @@ struct JSRuntime
     js::GCLocks         gcLocksHash;
     jsrefcount          gcKeepAtoms;
     uint32_t            gcBytes;
     uint32_t            gcTriggerBytes;
     size_t              gcLastBytes;
     size_t              gcMaxBytes;
     size_t              gcMaxMallocBytes;
     uint32_t            gcEmptyArenaPoolLifespan;
-    /* We access this without the GC lock, however a race will not affect correctness */
-    volatile uint32_t   gcNumFreeArenas;
+
+    /*
+     * Number of the committed arenas in all GC chunks including empty chunks.
+     * The counter is volatile as it is read without the GC lock, see comments
+     * in MaybeGC.
+     */
+    volatile uint32_t   gcNumArenasFreeCommitted;
     uint32_t            gcNumber;
     js::GCMarker        *gcIncrementalTracer;
     void                *gcVerifyData;
     bool                gcChunkAllocationSinceLastGC;
     int64_t             gcNextFullGCTime;
     int64_t             gcJitReleaseTime;
     JSGCMode            gcMode;
     volatile jsuword    gcBarrierFailed;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -452,16 +452,18 @@ ChunkPool::get(JSRuntime *rt)
         JS_ASSERT(emptyCount);
         emptyChunkListHead = chunk->info.next;
         --emptyCount;
     } else {
         JS_ASSERT(!emptyCount);
         chunk = Chunk::allocate(rt);
         if (!chunk)
             return NULL;
+        JS_ASSERT(chunk->info.numArenasFreeCommitted == ArenasPerChunk);
+        rt->gcNumArenasFreeCommitted += ArenasPerChunk;
     }
     JS_ASSERT(chunk->unused());
     JS_ASSERT(!rt->gcChunkSet.has(chunk));
 
 #ifdef JS_THREADSAFE
     if (wantBackgroundAllocation(rt))
         rt->gcHelperThread.startBackgroundAllocationIfIdle();
 #endif
@@ -553,50 +555,52 @@ ChunkPool::countCleanDecommittedArenas(J
 }
 
 /* static */ Chunk *
 Chunk::allocate(JSRuntime *rt)
 {
     Chunk *chunk = static_cast<Chunk *>(AllocChunk());
     if (!chunk)
         return NULL;
-    chunk->init(rt);
+    chunk->init();
     rt->gcStats.count(gcstats::STAT_NEW_CHUNK);
     return chunk;
 }
 
+/* Must be called with the GC lock taken. */
 /* static */ inline void
 Chunk::release(JSRuntime *rt, Chunk *chunk)
 {
     JS_ASSERT(chunk);
+    JS_ASSERT(rt->gcNumArenasFreeCommitted >= chunk->info.numArenasFreeCommitted);
+    rt->gcNumArenasFreeCommitted -= chunk->info.numArenasFreeCommitted;
     rt->gcStats.count(gcstats::STAT_DESTROY_CHUNK);
     FreeChunk(chunk);
 }
 
 void
-Chunk::init(JSRuntime *rt)
+Chunk::init()
 {
     JS_POISON(this, JS_FREE_PATTERN, ChunkSize);
 
     /*
      * We clear the bitmap to guard against xpc_IsGrayGCThing being called on
      * uninitialized data, which would happen before the first GC cycle.
      */
     bitmap.clear();
 
-    /* Initialize the arena tracking bitmap. */ 
+    /* Initialize the arena tracking bitmap. */
     decommittedArenas.clear(false);
 
     /* Initialize the chunk info. */
     info.freeArenasHead = &arenas[0].aheader;
     info.lastDecommittedArenaOffset = 0;
     info.numArenasFree = ArenasPerChunk;
     info.numArenasFreeCommitted = ArenasPerChunk;
     info.age = 0;
-    rt->gcNumFreeArenas += ArenasPerChunk;
 
     /* Initialize the arena header state. */
     for (jsuint i = 0; i < ArenasPerChunk; i++) {
         arenas[i].aheader.setAsNotAllocated();
         arenas[i].aheader.next = (i + 1 < ArenasPerChunk)
                                  ? &arenas[i + 1].aheader
                                  : NULL;
     }
@@ -638,17 +642,17 @@ Chunk::removeFromAvailableList()
         JS_ASSERT(info.next->info.prevp == &info.next);
         info.next->info.prevp = info.prevp;
     }
     info.prevp = NULL;
     info.next = NULL;
 }
 
 /*
- * Search for and return the next decommitted Arena. Our goal is to keep 
+ * Search for and return the next decommitted Arena. Our goal is to keep
  * lastDecommittedArenaOffset "close" to a free arena. We do this by setting
  * it to the most recently freed arena when we free, and forcing it to
  * the last alloc + 1 when we allocate.
  */
 jsuint
 Chunk::findDecommittedArenaOffset()
 {
     /* Note: lastFreeArenaOffset can be past the end of the list. */
@@ -660,17 +664,18 @@ Chunk::findDecommittedArenaOffset()
             return i;
     JS_NOT_REACHED("No decommitted arenas found.");
     return -1;
 }
 
 ArenaHeader *
 Chunk::fetchNextDecommittedArena()
 {
-    JS_ASSERT(info.numArenasFreeCommitted < info.numArenasFree);
+    JS_ASSERT(info.numArenasFreeCommitted == 0);
+    JS_ASSERT(info.numArenasFree > 0);
 
     jsuint offset = findDecommittedArenaOffset();
     info.lastDecommittedArenaOffset = offset + 1;
     --info.numArenasFree;
     decommittedArenas.unset(offset);
 
     Arena *arena = &arenas[offset];
     CommitMemory(arena, ArenaSize);
@@ -678,22 +683,24 @@ Chunk::fetchNextDecommittedArena()
 
     return &arena->aheader;
 }
 
 inline ArenaHeader *
 Chunk::fetchNextFreeArena(JSRuntime *rt)
 {
     JS_ASSERT(info.numArenasFreeCommitted > 0);
+    JS_ASSERT(info.numArenasFreeCommitted <= info.numArenasFree);
+    JS_ASSERT(info.numArenasFreeCommitted <= rt->gcNumArenasFreeCommitted);
 
     ArenaHeader *aheader = info.freeArenasHead;
     info.freeArenasHead = aheader->next;
     --info.numArenasFreeCommitted;
     --info.numArenasFree;
-    --rt->gcNumFreeArenas;
+    --rt->gcNumArenasFreeCommitted;
 
     return aheader;
 }
 
 ArenaHeader *
 Chunk::allocateArena(JSCompartment *comp, AllocKind thingKind)
 {
     JS_ASSERT(!noAvailableArenas());
@@ -741,17 +748,17 @@ Chunk::releaseArena(ArenaHeader *aheader
     JS_ATOMIC_ADD(&rt->gcBytes, -int32_t(ArenaSize));
     JS_ATOMIC_ADD(&comp->gcBytes, -int32_t(ArenaSize));
 
     aheader->setAsNotAllocated();
     aheader->next = info.freeArenasHead;
     info.freeArenasHead = aheader;
     ++info.numArenasFreeCommitted;
     ++info.numArenasFree;
-    ++rt->gcNumFreeArenas;
+    ++rt->gcNumArenasFreeCommitted;
 
     if (info.numArenasFree == 1) {
         JS_ASSERT(!info.prevp);
         JS_ASSERT(!info.next);
         addToAvailableList(comp);
     } else if (!unused()) {
         JS_ASSERT(info.prevp);
     } else {
@@ -2167,25 +2174,29 @@ MaybeGC(JSContext *cx)
     }
 
     if (comp->gcBytes > 8192 && comp->gcBytes >= 3 * (comp->gcTriggerBytes / 4)) {
         js_GC(cx, (rt->gcMode == JSGC_MODE_COMPARTMENT) ? comp : NULL, GC_NORMAL, gcstats::MAYBEGC);
         return;
     }
 
     /*
-     * On 32 bit setting gcNextFullGCTime below is not atomic and a race condition
-     * could trigger an GC. We tolerate this.
+     * Access to the counters and, on 32 bit, setting gcNextFullGCTime below
+     * is not atomic and a race condition could trigger or suppress the GC. We
+     * tolerate this.
      */
     int64_t now = PRMJ_Now();
     if (rt->gcNextFullGCTime && rt->gcNextFullGCTime <= now) {
-        if (rt->gcChunkAllocationSinceLastGC || rt->gcNumFreeArenas > MaxFreeCommittedArenas)
+        if (rt->gcChunkAllocationSinceLastGC ||
+            rt->gcNumArenasFreeCommitted > FreeCommittedArenasThreshold)
+        {
             js_GC(cx, NULL, GC_SHRINK, gcstats::MAYBEGC);
-        else
+        } else {
             rt->gcNextFullGCTime = now + GC_IDLE_FULL_SPAN;
+        }
     }
 }
 
 } /* namespace js */
 
 #ifdef JS_THREADSAFE
 
 namespace js {
@@ -2268,16 +2279,18 @@ GCHelperThread::threadLoop()
                 {
                     AutoUnlockGC unlock(rt);
                     chunk = Chunk::allocate(rt);
                 }
 
                 /* OOM stops the background allocation. */
                 if (!chunk)
                     break;
+                JS_ASSERT(chunk->info.numArenasFreeCommitted == ArenasPerChunk);
+                rt->gcNumArenasFreeCommitted += ArenasPerChunk;
                 rt->gcChunkPool.put(rt, chunk);
             } while (state == ALLOCATING && rt->gcChunkPool.wantBackgroundAllocation(rt));
             if (state == ALLOCATING)
                 state = IDLE;
             break;
           case CANCEL_ALLOCATION:
             state = IDLE;
             PR_NotifyAllCondVar(done);
@@ -2438,17 +2451,17 @@ DecommitFreePages(JSContext *cx)
                 aheader->next = chunk->info.freeArenasHead;
                 chunk->info.freeArenasHead = aheader;
                 continue;
             }
 
             size_t arenaIndex = Chunk::arenaIndex(aheader->arenaAddress());
             chunk->decommittedArenas.set(arenaIndex);
             --chunk->info.numArenasFreeCommitted;
-            --rt->gcNumFreeArenas;
+            --rt->gcNumArenasFreeCommitted;
 
             aheader = next;
         }
     }
 }
 
 static void
 SweepCompartments(JSContext *cx, JSGCInvocationKind gckind)
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -94,17 +94,17 @@ const size_t MAX_BACKGROUND_FINALIZE_KIN
 const size_t ArenaShift = 12;
 const size_t ArenaSize = size_t(1) << ArenaShift;
 const size_t ArenaMask = ArenaSize - 1;
 
 /*
  * This is the maximum number of arenas we allow in the FreeCommitted state
  * before we trigger a GC_SHRINK to release free arenas to the OS.
  */
-const static uint32_t MaxFreeCommittedArenas = (32 << 20) / ArenaSize;
+const static uint32_t FreeCommittedArenasThreshold = (32 << 20) / ArenaSize;
 
 /*
  * The mark bitmap has one bit per each GC cell. For multi-cell GC things this
  * wastes space but allows to avoid expensive devisions by thing's size when
  * accessing the bitmap. In addition this allows to use some bits for colored
  * marking during the cycle GC.
  */
 const size_t ArenaCellCount = size_t(1) << (ArenaShift - Cell::CellShift);
@@ -741,20 +741,22 @@ struct Chunk {
     inline void addToAvailableList(JSCompartment *compartment);
     inline void removeFromAvailableList();
 
     ArenaHeader *allocateArena(JSCompartment *comp, AllocKind kind);
 
     void releaseArena(ArenaHeader *aheader);
 
     static Chunk *allocate(JSRuntime *rt);
+
+    /* Must be called with the GC lock taken. */
     static inline void release(JSRuntime *rt, Chunk *chunk);
 
   private:
-    inline void init(JSRuntime *rt);
+    inline void init();
 
     /* Search for a decommitted arena to allocate. */
     jsuint findDecommittedArenaOffset();
     ArenaHeader* fetchNextDecommittedArena();
 
     /* Unlink and return the freeArenasHead. */
     inline ArenaHeader* fetchNextFreeArena(JSRuntime *rt);
 };