Reviewed by Eric.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2007 07:23:25 +0000 (07:23 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Nov 2007 07:23:25 +0000 (07:23 +0000)
        - only collect when the heap is full, unless we have lots of extra cost garbage

        1.1% SunSpider speedup.

        This shouldn't hit memory use much since the extra space in those
        blocks hangs around either way.

        * kjs/collector.cpp:
        (KJS::Collector::heapAllocate):
        (KJS::Collector::collect): Fix logic error that reversed the sense of collect's
        return value.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@27503 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/kjs/collector.cpp

index 72aef4cdf5ee3406a765b2af818fc49ac9182d8a..09268af0299661113a6dde74ba738523a29c5bbd 100644 (file)
@@ -1,3 +1,19 @@
+2007-11-06  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Eric.        
+        
+        - only collect when the heap is full, unless we have lots of extra cost garbage
+        
+        1.1% SunSpider speedup.
+        
+        This shouldn't hit memory use much since the extra space in those
+        blocks hangs around either way.
+
+        * kjs/collector.cpp:
+        (KJS::Collector::heapAllocate):
+        (KJS::Collector::collect): Fix logic error that reversed the sense of collect's 
+        return value.
+
 2007-11-06  Oliver Hunt  <oliver@apple.com>
 
         Reviewed by Maciej.
index 5c1f20976be780f83c148d122dd8b1105dc58e5b..bc2ccd59390d5963c1ec3bc3d8f17e369709aee1 100644 (file)
@@ -208,28 +208,27 @@ template <Collector::HeapType heapType> void* Collector::heapAllocate(size_t s)
   // don't spend any time debugging cases where we allocate inside an object's
   // deallocation code.
 
-  // collect if needed
   size_t numLiveObjects = heap.numLiveObjects;
-  size_t numLiveObjectsAtLastCollect = heap.numLiveObjectsAtLastCollect;
-  size_t numNewObjects = numLiveObjects - numLiveObjectsAtLastCollect;
-  const size_t newCost = heapType == PrimaryHeap ? numNewObjects + heap.extraCost : numNewObjects;
+  size_t usedBlocks = heap.usedBlocks;
+  size_t i = heap.firstBlockWithPossibleSpace;
 
-  if (newCost >= ALLOCATIONS_PER_COLLECTION && newCost >= numLiveObjectsAtLastCollect) {
-      collect();
-      numLiveObjects = heap.numLiveObjects;
+  // if we have a huge amount of extra cost, we'll try to collect even if we still have
+  // free cells left.
+  if (heapType == PrimaryHeap && heap.extraCost > ALLOCATIONS_PER_COLLECTION) {
+      size_t numLiveObjectsAtLastCollect = heap.numLiveObjectsAtLastCollect;
+      size_t numNewObjects = numLiveObjects - numLiveObjectsAtLastCollect;
+      const size_t newCost = heapType == PrimaryHeap ? numNewObjects + heap.extraCost : numNewObjects;
+      if (newCost >= ALLOCATIONS_PER_COLLECTION && newCost >= numLiveObjectsAtLastCollect)
+          goto collect;
   }
-  
+
   ASSERT(heap.operationInProgress == NoOperation);
 #ifndef NDEBUG
   // FIXME: Consider doing this in NDEBUG builds too (see comment above).
   heap.operationInProgress = Allocation;
 #endif
-  
-  // slab allocator
-  
-  size_t usedBlocks = heap.usedBlocks;
 
-  size_t i = heap.firstBlockWithPossibleSpace;
+scan:
   Block* targetBlock;
   size_t targetBlockUsedCells;
   if (i != usedBlocks) {
@@ -238,15 +237,35 @@ template <Collector::HeapType heapType> void* Collector::heapAllocate(size_t s)
     ASSERT(targetBlockUsedCells <= HeapConstants<heapType>::cellsPerBlock);
     while (targetBlockUsedCells == HeapConstants<heapType>::cellsPerBlock) {
       if (++i == usedBlocks)
-        goto allocateNewBlock;
+        goto collect;
       targetBlock = (Block*)heap.blocks[i];
       targetBlockUsedCells = targetBlock->usedCells;
       ASSERT(targetBlockUsedCells <= HeapConstants<heapType>::cellsPerBlock);
     }
     heap.firstBlockWithPossibleSpace = i;
   } else {
-allocateNewBlock:
-    // didn't find one, need to allocate a new block
+collect:
+    size_t numLiveObjectsAtLastCollect = heap.numLiveObjectsAtLastCollect;
+    size_t numNewObjects = numLiveObjects - numLiveObjectsAtLastCollect;
+    const size_t newCost = heapType == PrimaryHeap ? numNewObjects + heap.extraCost : numNewObjects;
+      
+    if (newCost >= ALLOCATIONS_PER_COLLECTION && newCost >= numLiveObjectsAtLastCollect) {
+#ifndef NDEBUG
+      heap.operationInProgress = NoOperation;
+#endif
+      bool collected = collect();
+#ifndef NDEBUG
+      heap.operationInProgress = Allocation;
+#endif
+      if (collected) {
+        numLiveObjects = heap.numLiveObjects;
+        usedBlocks = heap.usedBlocks;
+        i = heap.firstBlockWithPossibleSpace;
+        goto scan;
+      }
+    }
+  
+    // didn't find a block, and GC didn't reclaim anything, need to allocate a new block
     size_t numBlocks = heap.numBlocks;
     if (usedBlocks == numBlocks) {
       numBlocks = max(MIN_ARRAY_SIZE, numBlocks * GROWTH_FACTOR);
@@ -968,7 +987,7 @@ bool Collector::collect()
       reportOutOfMemoryToAllInterpreters();
   memoryFull = newMemoryFull;
 
-  return originalLiveObjects < numLiveObjects;
+  return numLiveObjects < originalLiveObjects;
 }
 
 size_t Collector::size()