Backed out changeset cd1252510d17 - I landed the wrong patch for bug 711623

js/src/jsapi.cpp

@@ -651,7 +651,7 @@ JSRuntime::JSRuntime()
     gcMaxBytes(0),
     gcMaxMallocBytes(0),
     gcEmptyArenaPoolLifespan(0),
-    gcNumArenasFreeCommitted(0),
+    gcNumFreeArenas(0),
     gcNumber(0),
     gcIncrementalTracer(NULL),
     gcVerifyData(NULL),

js/src/jscntxt.h

@@ -424,12 +424,8 @@ struct JSRuntime
     size_t              gcMaxBytes;
     size_t              gcMaxMallocBytes;
     uint32_t            gcEmptyArenaPoolLifespan;
-
-    /*
-     * Number of the committed arenas in non-empty chunks. The counter is
-     * volatile as it is read without the GC lock, see comments in MaybeGC.
-     */
-    volatile uint32_t   gcNumArenasFreeCommitted;
+    /* We access this without the GC lock, however a race will not affect correctness */
+    volatile uint32_t   gcNumFreeArenas;
     uint32_t            gcNumber;
     js::GCMarker        *gcIncrementalTracer;
     void                *gcVerifyData;

js/src/jsgc.cpp

@@ -558,7 +558,7 @@ Chunk::allocate(JSRuntime *rt)
     Chunk *chunk = static_cast<Chunk *>(AllocChunk());
     if (!chunk)
         return NULL;
-    chunk->init();
+    chunk->init(rt);
     rt->gcStats.count(gcstats::STAT_NEW_CHUNK);
     return chunk;
 }
@@ -572,7 +572,7 @@ Chunk::release(JSRuntime *rt, Chunk *chunk)
 }
 
 void
-Chunk::init()
+Chunk::init(JSRuntime *rt)
 {
     JS_POISON(this, JS_FREE_PATTERN, ChunkSize);
 
@@ -582,7 +582,7 @@ Chunk::init()
      */
     bitmap.clear();
 
-    /* Initialize the arena tracking bitmap. */
+    /* Initialize the arena tracking bitmap. */ 
     decommittedArenas.clear(false);
 
     /* Initialize the chunk info. */
@@ -591,6 +591,7 @@ Chunk::init()
     info.numArenasFree = ArenasPerChunk;
     info.numArenasFreeCommitted = ArenasPerChunk;
     info.age = 0;
+    rt->gcNumFreeArenas += ArenasPerChunk;
 
     /* Initialize the arena header state. */
     for (jsuint i = 0; i < ArenasPerChunk; i++) {
@@ -642,7 +643,7 @@ Chunk::removeFromAvailableList()
 }
 
 /*
- * 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.
@@ -664,8 +665,7 @@ Chunk::findDecommittedArenaOffset()
 ArenaHeader *
 Chunk::fetchNextDecommittedArena()
 {
-    JS_ASSERT(info.numArenasFreeCommitted == 0);
-    JS_ASSERT(info.numArenasFree > 0);
+    JS_ASSERT(info.numArenasFreeCommitted < info.numArenasFree);
 
     jsuint offset = findDecommittedArenaOffset();
     info.lastDecommittedArenaOffset = offset + 1;
@@ -683,14 +683,12 @@ 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->gcNumArenasFreeCommitted;
+    --rt->gcNumFreeArenas;
 
     return aheader;
 }
@@ -748,7 +746,7 @@ Chunk::releaseArena(ArenaHeader *aheader)
     info.freeArenasHead = aheader;
     ++info.numArenasFreeCommitted;
     ++info.numArenasFree;
-    ++rt->gcNumArenasFreeCommitted;
+    ++rt->gcNumFreeArenas;
 
     if (info.numArenasFree == 1) {
         JS_ASSERT(!info.prevp);
@@ -759,8 +757,6 @@ Chunk::releaseArena(ArenaHeader *aheader)
     } else {
         rt->gcChunkSet.remove(this);
         removeFromAvailableList();
-        JS_ASSERT(rt->gcNumArenasFreeCommitted >= ArenasPerChunk);
-        rt->gcNumArenasFreeCommitted -= ArenasPerChunk;
         rt->gcChunkPool.put(rt, this);
     }
 }
@@ -795,9 +791,6 @@ PickChunk(JSCompartment *comp)
         return NULL;
     }
 
-    JS_ASSERT(chunk->info.numArenasFreeCommitted == ArenasPerChunk);
-    rt->gcNumArenasFreeCommitted += ArenasPerChunk;
-
     chunk->info.prevp = NULL;
     chunk->info.next = NULL;
     chunk->addToAvailableList(comp);
@@ -2179,20 +2172,15 @@ MaybeGC(JSContext *cx)
     }
 
     /*
-     * 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.
+     * On 32 bit setting gcNextFullGCTime below is not atomic and a race condition
+     * could trigger an GC. We tolerate this.
      */
     int64_t now = PRMJ_Now();
     if (rt->gcNextFullGCTime && rt->gcNextFullGCTime <= now) {
-        if (rt->gcChunkAllocationSinceLastGC ||
-            rt->gcNumArenasFreeCommitted + rt->gcChunkPool.getEmptyCount() * ArenasPerChunk >
-            FreeCommittedArenasThreshold)
-        {
+        if (rt->gcChunkAllocationSinceLastGC || rt->gcNumFreeArenas > MaxFreeCommittedArenas)
             js_GC(cx, NULL, GC_SHRINK, gcstats::MAYBEGC);
-        } else {
+        else
             rt->gcNextFullGCTime = now + GC_IDLE_FULL_SPAN;
-        }
     }
 }
 
@@ -2285,10 +2273,6 @@ GCHelperThread::threadLoop()
                 /* OOM stops the background allocation. */
                 if (!chunk)
                     break;
-
-                /* Increase the global counter under the lock. */
-                JS_ASSERT(chunk->info.numArenasFreeCommitted == ArenasPerChunk);
-                rt->gcNumArenasFreeCommitted += ArenasPerChunk;
                 rt->gcChunkPool.put(rt, chunk);
             } while (state == ALLOCATING && rt->gcChunkPool.wantBackgroundAllocation(rt));
             if (state == ALLOCATING)
@@ -2459,7 +2443,7 @@ DecommitFreePages(JSContext *cx)
             size_t arenaIndex = Chunk::arenaIndex(aheader->arenaAddress());
             chunk->decommittedArenas.set(arenaIndex);
             --chunk->info.numArenasFreeCommitted;
-            --rt->gcNumArenasFreeCommitted;
+            --rt->gcNumFreeArenas;
 
             aheader = next;
         }

js/src/jsgc.h

@@ -99,7 +99,7 @@ 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 FreeCommittedArenasThreshold = (32 << 20) / ArenaSize;
+const static uint32_t MaxFreeCommittedArenas = (32 << 20) / ArenaSize;
 
 /*
  * The mark bitmap has one bit per each GC cell. For multi-cell GC things this
@@ -749,7 +749,7 @@ struct Chunk {
     static inline void release(JSRuntime *rt, Chunk *chunk);
 
   private:
-    inline void init();
+    inline void init(JSRuntime *rt);
 
     /* Search for a decommitted arena to allocate. */
     jsuint findDecommittedArenaOffset();