From 6ab46a19bac261f42b664c62f8c2477b294b86ea Mon Sep 17 00:00:00 2001 From: Balazs Kilvady Date: Thu, 4 Apr 2013 13:51:09 +0200 Subject: r134080 causes heap problem on linux systems where PAGESIZE != 4096 https://bugs.webkit.org/show_bug.cgi?id=102828 Patch by Balazs Kilvady on 2013-01-18 Reviewed by Mark Hahnenberg. Make MarkStackSegment::blockSize as the capacity of segments of a MarkStackArray. * JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCoreExports.def: * heap/MarkStack.cpp: (JSC): (JSC::MarkStackArray::MarkStackArray): (JSC::MarkStackArray::expand): (JSC::MarkStackArray::donateSomeCellsTo): (JSC::MarkStackArray::stealSomeCellsFrom): * heap/MarkStack.h: (JSC::MarkStackSegment::data): (CapacityFromSize): (MarkStackArray): * heap/MarkStackInlines.h: (JSC::MarkStackArray::setTopForFullSegment): (JSC::MarkStackArray::append): (JSC::MarkStackArray::isEmpty): (JSC::MarkStackArray::size): * runtime/Options.h: (JSC): Change-Id: I4663100b6b8b054bed03c0c6eb01bb9274a1b264 git-svn-id: http://svn.webkit.org/repository/webkit/trunk@140195 268f45cc-cd09-0410-ab3c-d52691b4dbfc Reviewed-by: Jocelyn Turcotte --- Source/JavaScriptCore/heap/MarkStack.cpp | 12 ++++-------- Source/JavaScriptCore/heap/MarkStack.h | 16 +++++----------- Source/JavaScriptCore/heap/MarkStackInlines.h | 10 +++++----- 3 files changed, 14 insertions(+), 24 deletions(-) (limited to 'Source/JavaScriptCore/heap') diff --git a/Source/JavaScriptCore/heap/MarkStack.cpp b/Source/JavaScriptCore/heap/MarkStack.cpp index 755a0ad50..39907c715 100644 --- a/Source/JavaScriptCore/heap/MarkStack.cpp +++ b/Source/JavaScriptCore/heap/MarkStack.cpp @@ -31,7 +31,6 @@ #include "CopiedSpace.h" #include "CopiedSpaceInlines.h" #include "Heap.h" -#include "Options.h" #include "JSArray.h" #include "JSCell.h" #include "JSObject.h" @@ -45,13 +44,13 @@ namespace JSC { +COMPILE_ASSERT(MarkStackSegment::blockSize == WeakBlock::blockSize, blockSizeMatch); + MarkStackArray::MarkStackArray(BlockAllocator& blockAllocator) : m_blockAllocator(blockAllocator) - , m_segmentCapacity(MarkStackSegment::capacityFromSize(Options::gcMarkStackSegmentSize())) , m_top(0) , m_numberOfSegments(0) { - ASSERT(MarkStackSegment::blockSize == WeakBlock::blockSize); m_segments.push(MarkStackSegment::create(m_blockAllocator.allocate())); m_numberOfSegments++; } @@ -64,7 +63,7 @@ MarkStackArray::~MarkStackArray() void MarkStackArray::expand() { - ASSERT(m_segments.head()->m_top == m_segmentCapacity); + ASSERT(m_segments.head()->m_top == s_segmentCapacity); MarkStackSegment* nextSegment = MarkStackSegment::create(m_blockAllocator.allocate()); m_numberOfSegments++; @@ -97,8 +96,6 @@ void MarkStackArray::donateSomeCellsTo(MarkStackArray& other) // we prefer donating whole segments over donating individual cells, // even if this skews away from our 1 / 2 target. - ASSERT(m_segmentCapacity == other.m_segmentCapacity); - size_t segmentsToDonate = m_numberOfSegments / 2; // If we only have one segment (our head) we don't donate any segments. if (!segmentsToDonate) { @@ -141,7 +138,6 @@ void MarkStackArray::stealSomeCellsFrom(MarkStackArray& other, size_t idleThread // To reduce copying costs, we prefer stealing a whole segment over stealing // individual cells, even if this skews away from our 1 / N target. - ASSERT(m_segmentCapacity == other.m_segmentCapacity); validatePrevious(); other.validatePrevious(); @@ -151,7 +147,7 @@ void MarkStackArray::stealSomeCellsFrom(MarkStackArray& other, size_t idleThread MarkStackSegment* otherHead = other.m_segments.removeHead(); MarkStackSegment* myHead = m_segments.removeHead(); - ASSERT(other.m_segments.head()->m_top == m_segmentCapacity); + ASSERT(other.m_segments.head()->m_top == s_segmentCapacity); m_segments.push(other.m_segments.removeHead()); diff --git a/Source/JavaScriptCore/heap/MarkStack.h b/Source/JavaScriptCore/heap/MarkStack.h index 2a7f04450..c97b6a735 100644 --- a/Source/JavaScriptCore/heap/MarkStack.h +++ b/Source/JavaScriptCore/heap/MarkStack.h @@ -75,16 +75,6 @@ public: { return bitwise_cast(this + 1); } - - static size_t capacityFromSize(size_t size) - { - return (size - sizeof(MarkStackSegment)) / sizeof(const JSCell*); - } - - static size_t sizeFromCapacity(size_t capacity) - { - return sizeof(MarkStackSegment) + capacity * sizeof(const JSCell*); - } static const size_t blockSize = 4 * KB; @@ -111,6 +101,10 @@ public: bool isEmpty(); private: + template struct CapacityFromSize { + static const size_t value = (size - sizeof(MarkStackSegment)) / sizeof(const JSCell*); + }; + JS_EXPORT_PRIVATE void expand(); size_t postIncTop(); @@ -124,7 +118,7 @@ private: DoublyLinkedList m_segments; BlockAllocator& m_blockAllocator; - size_t m_segmentCapacity; + JS_EXPORT_PRIVATE static const size_t s_segmentCapacity = CapacityFromSize::value; size_t m_top; size_t m_numberOfSegments; diff --git a/Source/JavaScriptCore/heap/MarkStackInlines.h b/Source/JavaScriptCore/heap/MarkStackInlines.h index 1595e843e..c577de602 100644 --- a/Source/JavaScriptCore/heap/MarkStackInlines.h +++ b/Source/JavaScriptCore/heap/MarkStackInlines.h @@ -52,8 +52,8 @@ inline size_t MarkStackArray::preDecTop() inline void MarkStackArray::setTopForFullSegment() { - ASSERT(m_segments.head()->m_top == m_segmentCapacity); - m_top = m_segmentCapacity; + ASSERT(m_segments.head()->m_top == s_segmentCapacity); + m_top = s_segmentCapacity; } inline void MarkStackArray::setTopForEmptySegment() @@ -82,7 +82,7 @@ inline void MarkStackArray::validatePrevious() inline void MarkStackArray::append(const JSCell* cell) { - if (m_top == m_segmentCapacity) + if (m_top == s_segmentCapacity) expand(); m_segments.head()->data()[postIncTop()] = cell; } @@ -102,7 +102,7 @@ inline bool MarkStackArray::isEmpty() if (m_top) return false; if (m_segments.head()->next()) { - ASSERT(m_segments.head()->next()->m_top == m_segmentCapacity); + ASSERT(m_segments.head()->next()->m_top == s_segmentCapacity); return false; } return true; @@ -110,7 +110,7 @@ inline bool MarkStackArray::isEmpty() inline size_t MarkStackArray::size() { - return m_top + m_segmentCapacity * (m_numberOfSegments - 1); + return m_top + s_segmentCapacity * (m_numberOfSegments - 1); } } // namespace JSC -- cgit v1.2.1 From 13ed0e19388202143b5a794754de1d0826f447a0 Mon Sep 17 00:00:00 2001 From: Mark Hahnenberg Date: Thu, 4 Apr 2013 14:31:14 +0200 Subject: WeakSet::removeAllocator leaks WeakBlocks https://bugs.webkit.org/show_bug.cgi?id=110228 Reviewed by Geoffrey Garen. We need to return the WeakBlock to the BlockAllocator after the call to WeakBlock::destroy. * heap/WeakSet.cpp: (JSC::WeakSet::removeAllocator): Change-Id: Iba6cff23e3d8b7a544a825dd1e435cf986b0d35f git-svn-id: http://svn.webkit.org/repository/webkit/trunk@143351 268f45cc-cd09-0410-ab3c-d52691b4dbfc Reviewed-by: Jocelyn Turcotte --- Source/JavaScriptCore/heap/WeakSet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Source/JavaScriptCore/heap') diff --git a/Source/JavaScriptCore/heap/WeakSet.cpp b/Source/JavaScriptCore/heap/WeakSet.cpp index 67b1d0613..7cedaee85 100644 --- a/Source/JavaScriptCore/heap/WeakSet.cpp +++ b/Source/JavaScriptCore/heap/WeakSet.cpp @@ -84,7 +84,7 @@ WeakBlock::FreeCell* WeakSet::addAllocator() void WeakSet::removeAllocator(WeakBlock* block) { m_blocks.remove(block); - WeakBlock::destroy(block); + heap()->blockAllocator().deallocate(WeakBlock::destroy(block)); } } // namespace JSC -- cgit v1.2.1