Title: [116484] trunk/Source/_javascript_Core
- Revision
- 116484
- Author
- [email protected]
- Date
- 2012-05-08 19:44:39 -0700 (Tue, 08 May 2012)
Log Message
Heap should not continually allocate new pages in steady state
https://bugs.webkit.org/show_bug.cgi?id=85936
Reviewed by Geoff Garen.
Currently, in steady state (i.e. a constant amount of live GC
memory with a constant rate of allocation) assuming we've just
finished a collection with X live blocks in CopiedSpace, we
increase our working set by X blocks in CopiedSpace with each
collection we perform. This is due to the fact that we allocate
until we run out of free blocks to use in the Heap before we
consider whether we should run a collection.
In the longer term, this issue will be mostly resolved by
implementing quick release for the CopiedSpace. In the shorter
term, we should change our policy to check whether we should
allocate before trying to use a free block from the Heap. We
can change our policy to something more appropriate once we
have implemented quick release.
This change should also have the convenient side effect of
reducing the variance in GC-heavy tests (e.g. v8-splay) due
to fact that we are doing less VM allocation during copying
collection. Overall, this patch is performance neutral across
the benchmarks we track.
* heap/CopiedSpace.cpp:
(JSC::CopiedSpace::getFreshBlock): Shuffle the request from the BlockAllocator
around so that we only do it if the block request must succeed
i.e. after we've already checked whether we should do a collection.
* heap/MarkedAllocator.cpp:
(JSC::MarkedAllocator::allocateSlowCase): Ditto.
(JSC::MarkedAllocator::allocateBlock): We no longer have a failure mode in this
function because by the time we've called it, we've already checked whether we
should run a collection so there's no point in returning null.
* heap/MarkedAllocator.h: Removing old arguments from function declaration.
(MarkedAllocator):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (116483 => 116484)
--- trunk/Source/_javascript_Core/ChangeLog 2012-05-09 02:37:56 UTC (rev 116483)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-05-09 02:44:39 UTC (rev 116484)
@@ -1,3 +1,43 @@
+2012-05-08 Mark Hahnenberg <[email protected]>
+
+ Heap should not continually allocate new pages in steady state
+ https://bugs.webkit.org/show_bug.cgi?id=85936
+
+ Reviewed by Geoff Garen.
+
+ Currently, in steady state (i.e. a constant amount of live GC
+ memory with a constant rate of allocation) assuming we've just
+ finished a collection with X live blocks in CopiedSpace, we
+ increase our working set by X blocks in CopiedSpace with each
+ collection we perform. This is due to the fact that we allocate
+ until we run out of free blocks to use in the Heap before we
+ consider whether we should run a collection.
+
+ In the longer term, this issue will be mostly resolved by
+ implementing quick release for the CopiedSpace. In the shorter
+ term, we should change our policy to check whether we should
+ allocate before trying to use a free block from the Heap. We
+ can change our policy to something more appropriate once we
+ have implemented quick release.
+
+ This change should also have the convenient side effect of
+ reducing the variance in GC-heavy tests (e.g. v8-splay) due
+ to fact that we are doing less VM allocation during copying
+ collection. Overall, this patch is performance neutral across
+ the benchmarks we track.
+
+ * heap/CopiedSpace.cpp:
+ (JSC::CopiedSpace::getFreshBlock): Shuffle the request from the BlockAllocator
+ around so that we only do it if the block request must succeed
+ i.e. after we've already checked whether we should do a collection.
+ * heap/MarkedAllocator.cpp:
+ (JSC::MarkedAllocator::allocateSlowCase): Ditto.
+ (JSC::MarkedAllocator::allocateBlock): We no longer have a failure mode in this
+ function because by the time we've called it, we've already checked whether we
+ should run a collection so there's no point in returning null.
+ * heap/MarkedAllocator.h: Removing old arguments from function declaration.
+ (MarkedAllocator):
+
2012-05-08 Gavin Barraclough <[email protected]>
SIGFPE on divide in classic interpreter
Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.cpp (116483 => 116484)
--- trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-05-09 02:37:56 UTC (rev 116483)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-05-09 02:44:39 UTC (rev 116484)
@@ -212,10 +212,10 @@
CheckedBoolean CopiedSpace::getFreshBlock(AllocationEffort allocationEffort, CopiedBlock** outBlock)
{
CopiedBlock* block = 0;
- if (HeapBlock* heapBlock = m_heap->blockAllocator().allocate())
- block = new (NotNull, heapBlock) CopiedBlock(heapBlock->m_allocation);
- else if (allocationEffort == AllocationMustSucceed) {
- if (!allocateNewBlock(&block)) {
+ if (allocationEffort == AllocationMustSucceed) {
+ if (HeapBlock* heapBlock = m_heap->blockAllocator().allocate())
+ block = new (NotNull, heapBlock) CopiedBlock(heapBlock->m_allocation);
+ else if (!allocateNewBlock(&block)) {
*outBlock = 0;
ASSERT_NOT_REACHED();
return false;
Modified: trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp (116483 => 116484)
--- trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp 2012-05-09 02:37:56 UTC (rev 116483)
+++ trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp 2012-05-09 02:44:39 UTC (rev 116484)
@@ -68,44 +68,30 @@
if (LIKELY(result != 0))
return result;
- AllocationEffort allocationEffort;
-
- if (m_heap->shouldCollect())
- allocationEffort = AllocationCanFail;
- else
- allocationEffort = AllocationMustSucceed;
-
- MarkedBlock* block = allocateBlock(allocationEffort);
- if (block) {
- addBlock(block);
- void* result = tryAllocate();
- ASSERT(result);
- return result;
+ if (m_heap->shouldCollect()) {
+ m_heap->collect(Heap::DoNotSweep);
+
+ result = tryAllocate();
+ if (result)
+ return result;
}
-
- m_heap->collect(Heap::DoNotSweep);
-
- result = tryAllocate();
-
- if (result)
- return result;
-
+
ASSERT(!m_heap->shouldCollect());
- addBlock(allocateBlock(AllocationMustSucceed));
-
+ MarkedBlock* block = allocateBlock();
+ ASSERT(block);
+ addBlock(block);
+
result = tryAllocate();
ASSERT(result);
return result;
}
-MarkedBlock* MarkedAllocator::allocateBlock(AllocationEffort allocationEffort)
+MarkedBlock* MarkedAllocator::allocateBlock()
{
MarkedBlock* block = static_cast<MarkedBlock*>(m_heap->blockAllocator().allocate());
if (block)
block = MarkedBlock::recycle(block, m_heap, m_cellSize, m_cellsNeedDestruction);
- else if (allocationEffort == AllocationCanFail)
- return 0;
else
block = MarkedBlock::create(m_heap, m_cellSize, m_cellsNeedDestruction);
Modified: trunk/Source/_javascript_Core/heap/MarkedAllocator.h (116483 => 116484)
--- trunk/Source/_javascript_Core/heap/MarkedAllocator.h 2012-05-09 02:37:56 UTC (rev 116483)
+++ trunk/Source/_javascript_Core/heap/MarkedAllocator.h 2012-05-09 02:44:39 UTC (rev 116484)
@@ -41,7 +41,7 @@
JS_EXPORT_PRIVATE void* allocateSlowCase();
void* tryAllocate();
void* tryAllocateHelper();
- MarkedBlock* allocateBlock(AllocationEffort);
+ MarkedBlock* allocateBlock();
MarkedBlock::FreeList m_freeList;
MarkedBlock* m_currentBlock;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
