Only Heap should be in charge of deciding how to select a subspace for a type
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2015 21:32:27 +0000 (21:32 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Mar 2015 21:32:27 +0000 (21:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142304

Reviewed by Mark Lam.

This slightly reduces the code duplication for selecting subspace based on type, and what
duplication is left is at least localized in HeapInlines.h. The immediate effect is that
the DFG and FTL don't have to duplicate this pattern.

* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::emitAllocateJSObject):
(JSC::DFG::SpeculativeJIT::emitAllocateVariableSizedJSObject):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::allocateObject):
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::allocateObjectOfType):
(JSC::Heap::subspaceForObjectOfType):
(JSC::Heap::allocatorForObjectOfType):
* runtime/JSCellInlines.h:
(JSC::allocateCell):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/HeapInlines.h
Source/JavaScriptCore/runtime/JSCellInlines.h

index 81c1c86..30dea39 100644 (file)
@@ -1,3 +1,27 @@
+2015-03-04  Filip Pizlo  <fpizlo@apple.com>
+
+        Only Heap should be in charge of deciding how to select a subspace for a type
+        https://bugs.webkit.org/show_bug.cgi?id=142304
+
+        Reviewed by Mark Lam.
+        
+        This slightly reduces the code duplication for selecting subspace based on type, and what
+        duplication is left is at least localized in HeapInlines.h. The immediate effect is that
+        the DFG and FTL don't have to duplicate this pattern.
+
+        * dfg/DFGSpeculativeJIT.h:
+        (JSC::DFG::SpeculativeJIT::emitAllocateJSObject):
+        (JSC::DFG::SpeculativeJIT::emitAllocateVariableSizedJSObject):
+        * ftl/FTLLowerDFGToLLVM.cpp:
+        (JSC::FTL::LowerDFGToLLVM::allocateObject):
+        * heap/Heap.h:
+        * heap/HeapInlines.h:
+        (JSC::Heap::allocateObjectOfType):
+        (JSC::Heap::subspaceForObjectOfType):
+        (JSC::Heap::allocatorForObjectOfType):
+        * runtime/JSCellInlines.h:
+        (JSC::allocateCell):
+
 2015-03-04  Andreas Kling  <akling@apple.com>
 
         Stale entries in WeakGCMaps are keeping tons of WeakBlocks alive unnecessarily.
index ce0849b..19d5bee 100644 (file)
@@ -2258,14 +2258,8 @@ public:
     void emitAllocateJSObject(GPRReg resultGPR, StructureType structure, StorageType storage,
         GPRReg scratchGPR1, GPRReg scratchGPR2, MacroAssembler::JumpList& slowPath)
     {
-        MarkedAllocator* allocator = 0;
         size_t size = ClassType::allocationSize(0);
-        if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
-            allocator = &m_jit.vm()->heap.allocatorForObjectWithImmortalStructureDestructor(size);
-        else if (ClassType::needsDestruction)
-            allocator = &m_jit.vm()->heap.allocatorForObjectWithNormalDestructor(size);
-        else
-            allocator = &m_jit.vm()->heap.allocatorForObjectWithoutDestructor(size);
+        MarkedAllocator* allocator = &m_jit.vm()->heap.allocatorForObjectOfType<ClassType>(size);
         m_jit.move(TrustedImmPtr(allocator), scratchGPR1);
         emitAllocateJSObject(resultGPR, scratchGPR1, structure, storage, scratchGPR2, slowPath);
     }
@@ -2276,25 +2270,19 @@ public:
         static_assert(!(MarkedSpace::preciseStep & (MarkedSpace::preciseStep - 1)), "MarkedSpace::preciseStep must be a power of two.");
         static_assert(!(MarkedSpace::impreciseStep & (MarkedSpace::impreciseStep - 1)), "MarkedSpace::impreciseStep must be a power of two.");
 
-        MarkedSpace::Subspace* subspace;
-        if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
-            subspace = &m_jit.vm()->heap.subspaceForObjectsWithImmortalStructure();
-        else if (ClassType::needsDestruction)
-            subspace = &m_jit.vm()->heap.subspaceForObjectNormalDestructor();
-        else
-            subspace = &m_jit.vm()->heap.subspaceForObjectWithoutDestructor();
+        MarkedSpace::Subspace& subspace = m_jit.vm()->heap.subspaceForObjectOfType<ClassType>();
         m_jit.add32(TrustedImm32(MarkedSpace::preciseStep - 1), allocationSize);
         MacroAssembler::Jump notSmall = m_jit.branch32(MacroAssembler::AboveOrEqual, allocationSize, TrustedImm32(MarkedSpace::preciseCutoff));
         m_jit.rshift32(allocationSize, TrustedImm32(getLSBSet(MarkedSpace::preciseStep)), scratchGPR1);
         m_jit.mul32(TrustedImm32(sizeof(MarkedAllocator)), scratchGPR1, scratchGPR1);
-        m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace->preciseAllocators[0]), scratchGPR1);
+        m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace.preciseAllocators[0]), scratchGPR1);
 
         MacroAssembler::Jump selectedSmallSpace = m_jit.jump();
         notSmall.link(&m_jit);
         slowPath.append(m_jit.branch32(MacroAssembler::AboveOrEqual, allocationSize, TrustedImm32(MarkedSpace::impreciseCutoff)));
         m_jit.rshift32(allocationSize, TrustedImm32(getLSBSet(MarkedSpace::impreciseStep)), scratchGPR1);
         m_jit.mul32(TrustedImm32(sizeof(MarkedAllocator)), scratchGPR1, scratchGPR1);
-        m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace->impreciseAllocators[0]), scratchGPR1);
+        m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace.impreciseAllocators[0]), scratchGPR1);
 
         selectedSmallSpace.link(&m_jit);
 
index ec67ad3..c1a24e0 100644 (file)
@@ -5225,14 +5225,8 @@ private:
     template<typename ClassType>
     LValue allocateObject(Structure* structure, LValue butterfly, LBasicBlock slowPath)
     {
-        MarkedAllocator* allocator;
         size_t size = ClassType::allocationSize(0);
-        if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
-            allocator = &vm().heap.allocatorForObjectWithImmortalStructureDestructor(size);
-        else if (ClassType::needsDestruction)
-            allocator = &vm().heap.allocatorForObjectWithNormalDestructor(size);
-        else
-            allocator = &vm().heap.allocatorForObjectWithoutDestructor(size);
+        MarkedAllocator* allocator = &vm().heap.allocatorForObjectOfType<ClassType>(size);
         return allocateObject(m_out.constIntPtr(allocator), structure, butterfly, slowPath);
     }
     
index e3812fd..19576c8 100644 (file)
@@ -140,9 +140,11 @@ public:
     MarkedSpace::Subspace& subspaceForObjectWithoutDestructor() { return m_objectSpace.subspaceForObjectsWithoutDestructor(); }
     MarkedSpace::Subspace& subspaceForObjectNormalDestructor() { return m_objectSpace.subspaceForObjectsWithNormalDestructor(); }
     MarkedSpace::Subspace& subspaceForObjectsWithImmortalStructure() { return m_objectSpace.subspaceForObjectsWithImmortalStructure(); }
+    template<typename ClassType> MarkedSpace::Subspace& subspaceForObjectOfType();
     MarkedAllocator& allocatorForObjectWithoutDestructor(size_t bytes) { return m_objectSpace.allocatorFor(bytes); }
     MarkedAllocator& allocatorForObjectWithNormalDestructor(size_t bytes) { return m_objectSpace.normalDestructorAllocatorFor(bytes); }
     MarkedAllocator& allocatorForObjectWithImmortalStructureDestructor(size_t bytes) { return m_objectSpace.immortalStructureDestructorAllocatorFor(bytes); }
+    template<typename ClassType> MarkedAllocator& allocatorForObjectOfType(size_t bytes);
     CopiedAllocator& storageAllocator() { return m_storageSpace.allocator(); }
     CheckedBoolean tryAllocateStorage(JSCell* intendedOwner, size_t, void**);
     CheckedBoolean tryReallocateStorage(JSCell* intendedOwner, void**, size_t, size_t);
@@ -259,6 +261,7 @@ private:
     void* allocateWithImmortalStructureDestructor(size_t); // For use with special objects whose Structures never die.
     void* allocateWithNormalDestructor(size_t); // For use with objects that inherit directly or indirectly from JSDestructibleObject.
     void* allocateWithoutDestructor(size_t); // For use with objects without destructors.
+    template<typename ClassType> void* allocateObjectOfType(size_t); // Chooses one of the methods above based on type.
 
     static const size_t minExtraCost = 256;
     static const size_t maxExtraCost = 1024 * 1024;
index 980508c..e08ec75 100644 (file)
@@ -205,6 +205,36 @@ inline void* Heap::allocateWithoutDestructor(size_t bytes)
     return m_objectSpace.allocateWithoutDestructor(bytes);
 }
 
+template<typename ClassType>
+void* Heap::allocateObjectOfType(size_t bytes)
+{
+    if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
+        return allocateWithImmortalStructureDestructor(bytes);
+    if (ClassType::needsDestruction)
+        return allocateWithNormalDestructor(bytes);
+    return allocateWithoutDestructor(bytes);
+}
+
+template<typename ClassType>
+MarkedSpace::Subspace& Heap::subspaceForObjectOfType()
+{
+    if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
+        return subspaceForObjectsWithImmortalStructure();
+    if (ClassType::needsDestruction)
+        return subspaceForObjectNormalDestructor();
+    return subspaceForObjectWithoutDestructor();
+}
+
+template<typename ClassType>
+MarkedAllocator& Heap::allocatorForObjectOfType(size_t bytes)
+{
+    if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
+        return allocatorForObjectWithImmortalStructureDestructor(bytes);
+    if (ClassType::needsDestruction)
+        return allocatorForObjectWithNormalDestructor(bytes);
+    return allocatorForObjectWithoutDestructor(bytes);
+}
+
 inline CheckedBoolean Heap::tryAllocateStorage(JSCell* intendedOwner, size_t bytes, void** outPtr)
 {
     CheckedBoolean result = m_storageSpace.tryAllocate(bytes, outPtr);
index b2fa406..dbb1004 100644 (file)
@@ -129,13 +129,7 @@ void* allocateCell(Heap& heap, size_t size)
 {
     ASSERT(!DisallowGC::isGCDisallowedOnCurrentThread());
     ASSERT(size >= sizeof(T));
-    JSCell* result = 0;
-    if (T::needsDestruction && T::hasImmortalStructure)
-        result = static_cast<JSCell*>(heap.allocateWithImmortalStructureDestructor(size));
-    else if (T::needsDestruction)
-        result = static_cast<JSCell*>(heap.allocateWithNormalDestructor(size));
-    else 
-        result = static_cast<JSCell*>(heap.allocateWithoutDestructor(size));
+    JSCell* result = static_cast<JSCell*>(heap.allocateObjectOfType<T>(size));
 #if ENABLE(GC_VALIDATION)
     ASSERT(!heap.vm()->isInitializingObject());
     heap.vm()->setInitializingObjectClass(T::info());