- Revision
- 120568
- Author
- [email protected]
- Date
- 2012-06-17 21:35:21 -0700 (Sun, 17 Jun 2012)
Log Message
GC copy phase spends needless cycles zero-filling blocks
https://bugs.webkit.org/show_bug.cgi?id=89128
Reviewed by Gavin Barraclough.
We only need to zero-fill when we're allocating memory that might not
get fully initialized before GC.
* heap/CopiedBlock.h:
(JSC::CopiedBlock::createNoZeroFill):
(JSC::CopiedBlock::create): Added a way to create without zero-filling.
This is our optimization.
(JSC::CopiedBlock::zeroFillToEnd):
(JSC::CopiedBlock::CopiedBlock): Split zero-filling out from creation,
so we can sometimes create without zero-filling.
* heap/CopiedSpace.cpp:
(JSC::CopiedSpace::init):
(JSC::CopiedSpace::tryAllocateSlowCase):
(JSC::CopiedSpace::doneCopying): Renamed addNewBlock to allocateBlock()
to clarify that the new block is always newly-allocated.
(JSC::CopiedSpace::doneFillingBlock): Make sure to zero-fill to the end
of a block that might be used in the future for allocation. (Most of the
time, this is a no-op, since we've already filled the block completely.)
(JSC::CopiedSpace::getFreshBlock): Removed this function because the
abstraction of "allocation must succeed" is no longer useful.
* heap/CopiedSpace.h: Updated declarations to match.
* heap/CopiedSpaceInlineMethods.h:
(JSC::CopiedSpace::allocateBlockForCopyingPhase): New function, which
knows that it can skip zero-filling.
Added tighter scoping to our lock, to improve parallelism.
(JSC::CopiedSpace::allocateBlock): Folded getFreshBlock functionality
into this function, for simplicity.
* heap/MarkStack.cpp:
(JSC::SlotVisitor::startCopying):
(JSC::SlotVisitor::allocateNewSpace): Use our new zero-fill-free helper
function for great good.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (120567 => 120568)
--- trunk/Source/_javascript_Core/ChangeLog 2012-06-18 04:18:46 UTC (rev 120567)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-06-18 04:35:21 UTC (rev 120568)
@@ -1,3 +1,51 @@
+2012-06-17 Geoffrey Garen <[email protected]>
+
+ GC copy phase spends needless cycles zero-filling blocks
+ https://bugs.webkit.org/show_bug.cgi?id=89128
+
+ Reviewed by Gavin Barraclough.
+
+ We only need to zero-fill when we're allocating memory that might not
+ get fully initialized before GC.
+
+ * heap/CopiedBlock.h:
+ (JSC::CopiedBlock::createNoZeroFill):
+ (JSC::CopiedBlock::create): Added a way to create without zero-filling.
+ This is our optimization.
+
+ (JSC::CopiedBlock::zeroFillToEnd):
+ (JSC::CopiedBlock::CopiedBlock): Split zero-filling out from creation,
+ so we can sometimes create without zero-filling.
+
+ * heap/CopiedSpace.cpp:
+ (JSC::CopiedSpace::init):
+ (JSC::CopiedSpace::tryAllocateSlowCase):
+ (JSC::CopiedSpace::doneCopying): Renamed addNewBlock to allocateBlock()
+ to clarify that the new block is always newly-allocated.
+
+ (JSC::CopiedSpace::doneFillingBlock): Make sure to zero-fill to the end
+ of a block that might be used in the future for allocation. (Most of the
+ time, this is a no-op, since we've already filled the block completely.)
+
+ (JSC::CopiedSpace::getFreshBlock): Removed this function because the
+ abstraction of "allocation must succeed" is no longer useful.
+
+ * heap/CopiedSpace.h: Updated declarations to match.
+
+ * heap/CopiedSpaceInlineMethods.h:
+ (JSC::CopiedSpace::allocateBlockForCopyingPhase): New function, which
+ knows that it can skip zero-filling.
+
+ Added tighter scoping to our lock, to improve parallelism.
+
+ (JSC::CopiedSpace::allocateBlock): Folded getFreshBlock functionality
+ into this function, for simplicity.
+
+ * heap/MarkStack.cpp:
+ (JSC::SlotVisitor::startCopying):
+ (JSC::SlotVisitor::allocateNewSpace): Use our new zero-fill-free helper
+ function for great good.
+
2012-06-17 Filip Pizlo <[email protected]>
DFG should attempt to use structure watchpoints for all inlined get_by_id's and put_by_id's
Modified: trunk/Source/_javascript_Core/heap/CopiedBlock.h (120567 => 120568)
--- trunk/Source/_javascript_Core/heap/CopiedBlock.h 2012-06-18 04:18:46 UTC (rev 120567)
+++ trunk/Source/_javascript_Core/heap/CopiedBlock.h 2012-06-18 04:35:21 UTC (rev 120568)
@@ -39,6 +39,7 @@
friend class CopiedAllocator;
public:
static CopiedBlock* create(const PageAllocationAligned&);
+ static CopiedBlock* createNoZeroFill(const PageAllocationAligned&);
static PageAllocationAligned destroy(CopiedBlock*);
char* payload();
@@ -47,16 +48,37 @@
private:
CopiedBlock(const PageAllocationAligned&);
+ void zeroFillToEnd(); // Can be called at any time to zero-fill to the end of the block.
void* m_offset;
uintptr_t m_isPinned;
};
-inline CopiedBlock* CopiedBlock::create(const PageAllocationAligned& allocation)
+inline CopiedBlock* CopiedBlock::createNoZeroFill(const PageAllocationAligned& allocation)
{
return new(NotNull, allocation.base()) CopiedBlock(allocation);
}
+inline CopiedBlock* CopiedBlock::create(const PageAllocationAligned& allocation)
+{
+ CopiedBlock* block = createNoZeroFill(allocation);
+ block->zeroFillToEnd();
+ return block;
+}
+
+inline void CopiedBlock::zeroFillToEnd()
+{
+#if USE(JSVALUE64)
+ char* offset = static_cast<char*>(m_offset);
+ memset(static_cast<void*>(offset), 0, static_cast<size_t>((reinterpret_cast<char*>(this) + m_allocation.size()) - offset));
+#else
+ JSValue emptyValue;
+ JSValue* limit = reinterpret_cast_ptr<JSValue*>(reinterpret_cast<char*>(this) + m_allocation.size());
+ for (JSValue* currentValue = reinterpret_cast<JSValue*>(m_offset); currentValue < limit; currentValue++)
+ *currentValue = emptyValue;
+#endif
+}
+
inline PageAllocationAligned CopiedBlock::destroy(CopiedBlock* block)
{
PageAllocationAligned allocation;
@@ -72,15 +94,6 @@
, m_isPinned(false)
{
ASSERT(is8ByteAligned(static_cast<void*>(m_offset)));
-#if USE(JSVALUE64)
- char* offset = static_cast<char*>(m_offset);
- memset(static_cast<void*>(offset), 0, static_cast<size_t>((reinterpret_cast<char*>(this) + allocation.size()) - offset));
-#else
- JSValue emptyValue;
- JSValue* limit = reinterpret_cast_ptr<JSValue*>(reinterpret_cast<char*>(this) + allocation.size());
- for (JSValue* currentValue = reinterpret_cast<JSValue*>(m_offset); currentValue < limit; currentValue++)
- *currentValue = emptyValue;
-#endif
}
inline char* CopiedBlock::payload()
Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.cpp (120567 => 120568)
--- trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-06-18 04:18:46 UTC (rev 120567)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-06-18 04:35:21 UTC (rev 120568)
@@ -58,8 +58,7 @@
m_toSpace = &m_blocks1;
m_fromSpace = &m_blocks2;
- if (!addNewBlock())
- CRASH();
+ allocateBlock();
}
CheckedBoolean CopiedSpace::tryAllocateSlowCase(size_t bytes, void** outPtr)
@@ -69,10 +68,8 @@
m_heap->didAllocate(m_allocator.currentCapacity());
- if (!addNewBlock()) {
- *outPtr = 0;
- return false;
- }
+ allocateBlock();
+
*outPtr = m_allocator.allocate(bytes);
ASSERT(*outPtr);
return true;
@@ -168,6 +165,8 @@
return;
}
+ block->zeroFillToEnd();
+
{
SpinLockHolder locker(&m_toSpaceLock);
m_toSpace->push(block);
@@ -223,35 +222,12 @@
curr = next;
}
- if (!m_toSpace->head()) {
- if (!addNewBlock())
- CRASH();
- } else
+ if (!m_toSpace->head())
+ allocateBlock();
+ else
m_allocator.resetCurrentBlock(static_cast<CopiedBlock*>(m_toSpace->head()));
}
-CheckedBoolean CopiedSpace::getFreshBlock(AllocationEffort allocationEffort, CopiedBlock** outBlock)
-{
- CopiedBlock* block = 0;
- if (allocationEffort == AllocationMustSucceed)
- block = CopiedBlock::create(m_heap->blockAllocator().allocate());
- else {
- ASSERT(allocationEffort == AllocationCanFail);
- if (m_heap->shouldCollect())
- m_heap->collect(Heap::DoNotSweep);
-
- if (!getFreshBlock(AllocationMustSucceed, &block)) {
- *outBlock = 0;
- ASSERT_NOT_REACHED();
- return false;
- }
- }
- ASSERT(block);
- ASSERT(is8ByteAligned(block->m_offset));
- *outBlock = block;
- return true;
-}
-
size_t CopiedSpace::size()
{
size_t calculatedSize = 0;
Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.h (120567 => 120568)
--- trunk/Source/_javascript_Core/heap/CopiedSpace.h 2012-06-18 04:18:46 UTC (rev 120567)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.h 2012-06-18 04:35:21 UTC (rev 120568)
@@ -77,22 +77,20 @@
static CopiedBlock* blockFor(void*);
private:
- CheckedBoolean tryAllocateSlowCase(size_t, void**);
- CheckedBoolean addNewBlock();
- CheckedBoolean allocateNewBlock(CopiedBlock**);
-
static void* allocateFromBlock(CopiedBlock*, size_t);
+ static bool isOversize(size_t);
+ static bool fitsInBlock(CopiedBlock*, size_t);
+ static CopiedBlock* oversizeBlockFor(void* ptr);
+
+ CheckedBoolean tryAllocateSlowCase(size_t, void**);
CheckedBoolean tryAllocateOversize(size_t, void**);
CheckedBoolean tryReallocateOversize(void**, size_t, size_t);
- static bool isOversize(size_t);
-
- CheckedBoolean borrowBlock(CopiedBlock**);
- CheckedBoolean getFreshBlock(AllocationEffort, CopiedBlock**);
+ void allocateBlock();
+ CopiedBlock* allocateBlockForCopyingPhase();
+
void doneFillingBlock(CopiedBlock*);
void recycleBlock(CopiedBlock*);
- static bool fitsInBlock(CopiedBlock*, size_t);
- static CopiedBlock* oversizeBlockFor(void* ptr);
Heap* m_heap;
Modified: trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h (120567 => 120568)
--- trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h 2012-06-18 04:18:46 UTC (rev 120567)
+++ trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h 2012-06-18 04:35:21 UTC (rev 120568)
@@ -84,48 +84,33 @@
}
}
-inline CheckedBoolean CopiedSpace::borrowBlock(CopiedBlock** outBlock)
+inline CopiedBlock* CopiedSpace::allocateBlockForCopyingPhase()
{
- CopiedBlock* block = 0;
- if (!getFreshBlock(AllocationMustSucceed, &block)) {
- *outBlock = 0;
- return false;
- }
-
ASSERT(m_inCopyingPhase);
- MutexLocker locker(m_loanedBlocksLock);
- m_numberOfLoanedBlocks++;
+ CopiedBlock* block = CopiedBlock::createNoZeroFill(m_heap->blockAllocator().allocate());
+ {
+ MutexLocker locker(m_loanedBlocksLock);
+ m_numberOfLoanedBlocks++;
+ }
+
ASSERT(block->m_offset == block->payload());
- *outBlock = block;
- return true;
+ return block;
}
-inline CheckedBoolean CopiedSpace::addNewBlock()
+inline void CopiedSpace::allocateBlock()
{
- CopiedBlock* block = 0;
- if (!getFreshBlock(AllocationCanFail, &block))
- return false;
+ if (m_heap->shouldCollect())
+ m_heap->collect(Heap::DoNotSweep);
+
+ CopiedBlock* block = CopiedBlock::create(m_heap->blockAllocator().allocate());
m_toSpace->push(block);
m_blockFilter.add(reinterpret_cast<Bits>(block));
m_blockSet.add(block);
m_allocator.resetCurrentBlock(block);
- return true;
}
-inline CheckedBoolean CopiedSpace::allocateNewBlock(CopiedBlock** outBlock)
-{
- PageAllocationAligned allocation = PageAllocationAligned::allocate(HeapBlock::s_blockSize, HeapBlock::s_blockSize, OSAllocator::JSGCHeapPages);
- if (!static_cast<bool>(allocation)) {
- *outBlock = 0;
- return false;
- }
-
- *outBlock = new (NotNull, allocation.base()) CopiedBlock(allocation);
- return true;
-}
-
inline bool CopiedSpace::fitsInBlock(CopiedBlock* block, size_t bytes)
{
return static_cast<char*>(block->m_offset) + bytes < reinterpret_cast<char*>(block) + block->capacity() && static_cast<char*>(block->m_offset) + bytes > block->m_offset;
Modified: trunk/Source/_javascript_Core/heap/MarkStack.cpp (120567 => 120568)
--- trunk/Source/_javascript_Core/heap/MarkStack.cpp 2012-06-18 04:18:46 UTC (rev 120567)
+++ trunk/Source/_javascript_Core/heap/MarkStack.cpp 2012-06-18 04:35:21 UTC (rev 120568)
@@ -495,8 +495,7 @@
void SlotVisitor::startCopying()
{
ASSERT(!m_copyBlock);
- if (!m_shared.m_copiedSpace->borrowBlock(&m_copyBlock))
- CRASH();
+ m_copyBlock = m_shared.m_copiedSpace->allocateBlockForCopyingPhase();
}
void* SlotVisitor::allocateNewSpace(void* ptr, size_t bytes)
@@ -517,8 +516,7 @@
// We don't need to lock across these two calls because the master thread won't
// call doneCopying() because this thread is considered active.
m_shared.m_copiedSpace->doneFillingBlock(m_copyBlock);
- if (!m_shared.m_copiedSpace->borrowBlock(&m_copyBlock))
- CRASH();
+ m_copyBlock = m_shared.m_copiedSpace->allocateBlockForCopyingPhase();
}
return CopiedSpace::allocateFromBlock(m_copyBlock, bytes);
}
