Changeset 90865 in webkit

Timestamp:

Jul 12, 2011 3:35:39 PM (13 years ago)

Author:

commit-queue@webkit.org

Message:

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

Patch by Filip Pizlo <​fpizlo@apple.com> 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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/Heap.cpp (modified) (5 diffs)
• heap/Heap.h (modified) (3 diffs)
• runtime/JSGlobalData.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-07-12  Filip Pizlo  <fpizlo@apple.com>
+
+        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  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -249 +249 @@
     , m_slotVisitor(globalData->jsArrayVPtr)
     , m_handleHeap(globalData)
+    , m_isSafeToCollect(false)
     , m_globalData(globalData)
 {
@@ -312 +313 @@
 }
 
+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)
 {
@@ -319 +328 @@
 #endif
 
-    m_operationInProgress = Allocation;
-    void* result = m_newSpace.allocate(sizeClass);
-    m_operationInProgress = NoOperation;
-
+    void* result = tryAllocate(sizeClass);
+
+    if (LIKELY(result != 0))
+        return result;
+
+    if (m_newSpace.waterMark() < m_newSpace.highWaterMark() || !m_isSafeToCollect) {
+        m_newSpace.addBlock(sizeClass, allocateBlock(sizeClass.cellSize));
+        void* result = tryAllocate(sizeClass);
+        ASSERT(result);
+        return result;
+    }
+
+    collect(DoNotSweep);
+    
+    result = tryAllocate(sizeClass);
+    
     if (result)
         return result;
-
-    if (m_newSpace.waterMark() < m_newSpace.highWaterMark()) {
-        m_newSpace.addBlock(sizeClass, allocateBlock(sizeClass.cellSize));
-        return allocate(sizeClass);
-    }
-
-    collect(DoNotSweep);
-    return allocate(sizeClass);
+    
+    ASSERT(m_newSpace.waterMark() < m_newSpace.highWaterMark());
+    
+    m_newSpace.addBlock(sizeClass, allocateBlock(sizeClass.cellSize));
+    
+    result = tryAllocate(sizeClass);
+    ASSERT(result);
+    return result;
 }
 
@@ -526 +547 @@
 void Heap::collectAllGarbage()
 {
+    if (!m_isSafeToCollect)
+        return;
     m_slotVisitor.setShouldUnlinkCalls(true);
     collect(DoSweep);
@@ -534 +557 @@
 {
     ASSERT(globalData()->identifierTable == wtfThreadData().currentIdentifierTable());
+    ASSERT(m_isSafeToCollect);
     JAVASCRIPTCORE_GC_BEGIN();
 

trunk/Source/JavaScriptCore/heap/Heap.h

@@ -83 +83 @@
         void* allocate(size_t);
         void* allocate(NewSpace::SizeClass&);
+        void notifyIsSafeToCollect() { m_isSafeToCollect = true; }
         void collectAllGarbage();
 
@@ -136 +137 @@
         void markTempSortVectors(HeapRootVisitor&);
 
+        void* tryAllocate(NewSpace::SizeClass&);
+        
         enum SweepToggle { DoNotSweep, DoSweep };
         void collect(SweepToggle);
@@ -161 +164 @@
         HandleHeap m_handleHeap;
         HandleStack m_handleStack;
+        
+        bool m_isSafeToCollect;
 
         JSGlobalData* m_globalData;

trunk/Source/JavaScriptCore/runtime/JSGlobalData.cpp

@@ -262 +262 @@
     jitStubs = adoptPtr(new JITThunks(this));
 #endif
+
+    heap.notifyIsSafeToCollect();
 }