[webkit-changes] [90865] trunk/Source/JavaScriptCore

Tue, 12 Jul 2011 15:35:44 -0700

Title: [90865] trunk/Source/_javascript_Core
Revision
90865
Author
[email protected]
Date
2011-07-12 15:35:39 -0700 (Tue, 12 Jul 2011)

Log Message

COLLECT_ON_EVERY_ALLOCATION no longer works.
https://bugs.webkit.org/show_bug.cgi?id=64388

Patch by Filip Pizlo <[email protected]> on 2011-07-12
Reviewed by Oliver Hunt.

Added a flag to Heap that determines if it's safe to collect (which for now means that
JSGlobalObject has actually been initialized, but it should work for other things, too).
This allows JSGlobalObject to allocate even if the allocator wants to GC; instead of
GCing it just grows the heap, if necessary.

Then changed Heap::allocate() to not recurse ad infinitum when
COLLECT_ON_EVERY_ALLOCATION is set.  This also makes the allocator generally more
resilient against bugs; this change allowed me to put in handy assertions, such as that
an allocation must succeed after either a collection or after a new block was added.

* heap/Heap.cpp:
(JSC::Heap::Heap):
(JSC::Heap::tryAllocate):
(JSC::Heap::allocate):
(JSC::Heap::collectAllGarbage):
(JSC::Heap::collect):
* heap/Heap.h:
(JSC::Heap::notifyIsSafeToCollect):
* runtime/JSGlobalData.cpp:
(JSC::JSGlobalData::JSGlobalData):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (90864 => 90865)


--- trunk/Source/_javascript_Core/ChangeLog	2011-07-12 22:35:28 UTC (rev 90864)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-07-12 22:35:39 UTC (rev 90865)
@@ -1,5 +1,33 @@
 2011-07-12  Filip Pizlo  <[email protected]>
 
+        COLLECT_ON_EVERY_ALLOCATION no longer works.
+        https://bugs.webkit.org/show_bug.cgi?id=64388
+
+        Reviewed by Oliver Hunt.
+        
+        Added a flag to Heap that determines if it's safe to collect (which for now means that
+        JSGlobalObject has actually been initialized, but it should work for other things, too).
+        This allows JSGlobalObject to allocate even if the allocator wants to GC; instead of
+        GCing it just grows the heap, if necessary.
+        
+        Then changed Heap::allocate() to not recurse ad infinitum when
+        COLLECT_ON_EVERY_ALLOCATION is set.  This also makes the allocator generally more
+        resilient against bugs; this change allowed me to put in handy assertions, such as that
+        an allocation must succeed after either a collection or after a new block was added.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::Heap):
+        (JSC::Heap::tryAllocate):
+        (JSC::Heap::allocate):
+        (JSC::Heap::collectAllGarbage):
+        (JSC::Heap::collect):
+        * heap/Heap.h:
+        (JSC::Heap::notifyIsSafeToCollect):
+        * runtime/JSGlobalData.cpp:
+        (JSC::JSGlobalData::JSGlobalData):
+
+2011-07-12  Filip Pizlo  <[email protected]>
+
         DFG JIT put_by_id transition caching does not inform the GC about the structure and
         prototype chain that it is referencing.
         https://bugs.webkit.org/show_bug.cgi?id=64387

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (90864 => 90865)


--- trunk/Source/_javascript_Core/heap/Heap.cpp	2011-07-12 22:35:28 UTC (rev 90864)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2011-07-12 22:35:39 UTC (rev 90865)
@@ -248,6 +248,7 @@
     , m_machineThreads(this)
     , m_slotVisitor(globalData->jsArrayVPtr)
     , m_handleHeap(globalData)
+    , m_isSafeToCollect(false)
     , m_globalData(globalData)
 {
     m_newSpace.setHighWaterMark(minBytesPerCycle);
@@ -311,6 +312,14 @@
     m_extraCost += cost;
 }
 
+inline void* Heap::tryAllocate(NewSpace::SizeClass& sizeClass)
+{
+    m_operationInProgress = Allocation;
+    void* result = m_newSpace.allocate(sizeClass);
+    m_operationInProgress = NoOperation;
+    return result;
+}
+
 void* Heap::allocate(NewSpace::SizeClass& sizeClass)
 {
 #if COLLECT_ON_EVERY_ALLOCATION
@@ -318,20 +327,32 @@
     ASSERT(m_operationInProgress == NoOperation);
 #endif
 
-    m_operationInProgress = Allocation;
-    void* result = m_newSpace.allocate(sizeClass);
-    m_operationInProgress = NoOperation;
+    void* result = tryAllocate(sizeClass);
 
-    if (result)
+    if (LIKELY(result != 0))
         return result;
 
-    if (m_newSpace.waterMark() < m_newSpace.highWaterMark()) {
+    if (m_newSpace.waterMark() < m_newSpace.highWaterMark() || !m_isSafeToCollect) {
         m_newSpace.addBlock(sizeClass, allocateBlock(sizeClass.cellSize));
-        return allocate(sizeClass);
+        void* result = tryAllocate(sizeClass);
+        ASSERT(result);
+        return result;
     }
 
     collect(DoNotSweep);
-    return allocate(sizeClass);
+    
+    result = tryAllocate(sizeClass);
+    
+    if (result)
+        return result;
+    
+    ASSERT(m_newSpace.waterMark() < m_newSpace.highWaterMark());
+    
+    m_newSpace.addBlock(sizeClass, allocateBlock(sizeClass.cellSize));
+    
+    result = tryAllocate(sizeClass);
+    ASSERT(result);
+    return result;
 }
 
 void Heap::protect(JSValue k)
@@ -525,6 +546,8 @@
 
 void Heap::collectAllGarbage()
 {
+    if (!m_isSafeToCollect)
+        return;
     m_slotVisitor.setShouldUnlinkCalls(true);
     collect(DoSweep);
     m_slotVisitor.setShouldUnlinkCalls(false);
@@ -533,6 +556,7 @@
 void Heap::collect(SweepToggle sweepToggle)
 {
     ASSERT(globalData()->identifierTable == wtfThreadData().currentIdentifierTable());
+    ASSERT(m_isSafeToCollect);
     _javascript_CORE_GC_BEGIN();
 
     markRoots();

Modified: trunk/Source/_javascript_Core/heap/Heap.h (90864 => 90865)


--- trunk/Source/_javascript_Core/heap/Heap.h	2011-07-12 22:35:28 UTC (rev 90864)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2011-07-12 22:35:39 UTC (rev 90865)
@@ -82,6 +82,7 @@
 
         void* allocate(size_t);
         void* allocate(NewSpace::SizeClass&);
+        void notifyIsSafeToCollect() { m_isSafeToCollect = true; }
         void collectAllGarbage();
 
         void reportExtraMemoryCost(size_t cost);
@@ -135,6 +136,8 @@
         void markProtectedObjects(HeapRootVisitor&);
         void markTempSortVectors(HeapRootVisitor&);
 
+        void* tryAllocate(NewSpace::SizeClass&);
+        
         enum SweepToggle { DoNotSweep, DoSweep };
         void collect(SweepToggle);
         void shrink();
@@ -160,6 +163,8 @@
         SlotVisitor m_slotVisitor;
         HandleHeap m_handleHeap;
         HandleStack m_handleStack;
+        
+        bool m_isSafeToCollect;
 
         JSGlobalData* m_globalData;
     };

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalData.cpp (90864 => 90865)


--- trunk/Source/_javascript_Core/runtime/JSGlobalData.cpp	2011-07-12 22:35:28 UTC (rev 90864)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalData.cpp	2011-07-12 22:35:39 UTC (rev 90865)
@@ -261,6 +261,8 @@
 #endif
     jitStubs = adoptPtr(new JITThunks(this));
 #endif
+
+    heap.notifyIsSafeToCollect();
 }
 
 void JSGlobalData::clearBuiltinStructures()

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to