Title: [118093] trunk/Source/_javascript_Core
- Revision
- 118093
- Author
- mhahnenb...@apple.com
- Date
- 2012-05-22 18:28:03 -0700 (Tue, 22 May 2012)
Log Message
CopiedSpace::contains doesn't check for oversize blocks
https://bugs.webkit.org/show_bug.cgi?id=87180
Reviewed by Geoffrey Garen.
When doing a conservative scan we use CopiedSpace::contains to determine if a particular
address points into the CopiedSpace. Currently contains() only checks if the address
points to a block in to-space, which means that pointers to oversize blocks may not get scanned.
* heap/CopiedSpace.cpp:
(JSC::CopiedSpace::tryAllocateOversize):
(JSC::CopiedSpace::tryReallocateOversize):
(JSC::CopiedSpace::doneFillingBlock):
(JSC::CopiedSpace::doneCopying):
* heap/CopiedSpace.h: Refactored CopiedSpace so that all blocks (oversize and to-space) are
in a single hash set and bloom filter for membership testing.
(CopiedSpace):
* heap/CopiedSpaceInlineMethods.h:
(JSC::CopiedSpace::contains): We check for the normal block first. Since the oversize blocks are
only page aligned, rather than block aligned, we have to re-mask the ptr to check if it's in
CopiedSpace. Also added a helper function of the same name that takes a CopiedBlock* and checks
if it's in CopiedSpace so that check isn't typed out twice.
(JSC):
(JSC::CopiedSpace::startedCopying):
(JSC::CopiedSpace::addNewBlock):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (118092 => 118093)
--- trunk/Source/_javascript_Core/ChangeLog 2012-05-23 01:13:03 UTC (rev 118092)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-05-23 01:28:03 UTC (rev 118093)
@@ -1,3 +1,31 @@
+2012-05-22 Mark Hahnenberg <mhahnenb...@apple.com>
+
+ CopiedSpace::contains doesn't check for oversize blocks
+ https://bugs.webkit.org/show_bug.cgi?id=87180
+
+ Reviewed by Geoffrey Garen.
+
+ When doing a conservative scan we use CopiedSpace::contains to determine if a particular
+ address points into the CopiedSpace. Currently contains() only checks if the address
+ points to a block in to-space, which means that pointers to oversize blocks may not get scanned.
+
+ * heap/CopiedSpace.cpp:
+ (JSC::CopiedSpace::tryAllocateOversize):
+ (JSC::CopiedSpace::tryReallocateOversize):
+ (JSC::CopiedSpace::doneFillingBlock):
+ (JSC::CopiedSpace::doneCopying):
+ * heap/CopiedSpace.h: Refactored CopiedSpace so that all blocks (oversize and to-space) are
+ in a single hash set and bloom filter for membership testing.
+ (CopiedSpace):
+ * heap/CopiedSpaceInlineMethods.h:
+ (JSC::CopiedSpace::contains): We check for the normal block first. Since the oversize blocks are
+ only page aligned, rather than block aligned, we have to re-mask the ptr to check if it's in
+ CopiedSpace. Also added a helper function of the same name that takes a CopiedBlock* and checks
+ if it's in CopiedSpace so that check isn't typed out twice.
+ (JSC):
+ (JSC::CopiedSpace::startedCopying):
+ (JSC::CopiedSpace::addNewBlock):
+
2012-05-22 Geoffrey Garen <gga...@apple.com>
CopiedBlock and MarkedBlock should have proper value semantics (i.e., destructors)
Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.cpp (118092 => 118093)
--- trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-05-23 01:13:03 UTC (rev 118092)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-05-23 01:28:03 UTC (rev 118093)
@@ -79,7 +79,8 @@
CopiedBlock* block = CopiedBlock::create(allocation);
m_oversizeBlocks.push(block);
- m_oversizeFilter.add(reinterpret_cast<Bits>(block));
+ m_blockFilter.add(reinterpret_cast<Bits>(block));
+ m_blockSet.add(block);
*outPtr = allocateFromBlock(block, bytes);
@@ -135,6 +136,7 @@
if (isOversize(oldSize)) {
CopiedBlock* oldBlock = oversizeBlockFor(oldPtr);
m_oversizeBlocks.remove(oldBlock);
+ m_blockSet.remove(oldBlock);
CopiedBlock::destroy(oldBlock).deallocate();
}
@@ -156,8 +158,8 @@
{
MutexLocker locker(m_toSpaceLock);
m_toSpace->push(block);
- m_toSpaceSet.add(block);
- m_toSpaceFilter.add(reinterpret_cast<Bits>(block));
+ m_blockSet.add(block);
+ m_blockFilter.add(reinterpret_cast<Bits>(block));
}
{
@@ -183,14 +185,14 @@
CopiedBlock* block = static_cast<CopiedBlock*>(m_fromSpace->removeHead());
if (block->m_isPinned) {
block->m_isPinned = false;
- // We don't add the block to the toSpaceSet because it was never removed.
- ASSERT(m_toSpaceSet.contains(block));
- m_toSpaceFilter.add(reinterpret_cast<Bits>(block));
+ // We don't add the block to the blockSet because it was never removed.
+ ASSERT(m_blockSet.contains(block));
+ m_blockFilter.add(reinterpret_cast<Bits>(block));
m_toSpace->push(block);
continue;
}
- m_toSpaceSet.remove(block);
+ m_blockSet.remove(block);
m_heap->blockAllocator().deallocate(CopiedBlock::destroy(block));
}
@@ -199,9 +201,12 @@
CopiedBlock* next = static_cast<CopiedBlock*>(curr->next());
if (!curr->m_isPinned) {
m_oversizeBlocks.remove(curr);
+ m_blockSet.remove(curr);
CopiedBlock::destroy(curr).deallocate();
- } else
+ } else {
+ m_blockFilter.add(reinterpret_cast<Bits>(curr));
curr->m_isPinned = false;
+ }
curr = next;
}
Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.h (118092 => 118093)
--- trunk/Source/_javascript_Core/heap/CopiedSpace.h 2012-05-23 01:13:03 UTC (rev 118092)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.h 2012-05-23 01:28:03 UTC (rev 118093)
@@ -64,6 +64,7 @@
void pin(CopiedBlock*);
bool isPinned(void*);
+ bool contains(CopiedBlock*);
bool contains(void*, CopiedBlock*&);
size_t size();
@@ -96,9 +97,8 @@
CopiedAllocator m_allocator;
- TinyBloomFilter m_toSpaceFilter;
- TinyBloomFilter m_oversizeFilter;
- HashSet<CopiedBlock*> m_toSpaceSet;
+ TinyBloomFilter m_blockFilter;
+ HashSet<CopiedBlock*> m_blockSet;
Mutex m_toSpaceLock;
Modified: trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h (118092 => 118093)
--- trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h 2012-05-23 01:13:03 UTC (rev 118092)
+++ trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h 2012-05-23 01:28:03 UTC (rev 118093)
@@ -35,11 +35,21 @@
namespace JSC {
+inline bool CopiedSpace::contains(CopiedBlock* block)
+{
+ return !m_blockFilter.ruleOut(reinterpret_cast<Bits>(block)) && m_blockSet.contains(block);
+}
+
inline bool CopiedSpace::contains(void* ptr, CopiedBlock*& result)
{
CopiedBlock* block = blockFor(ptr);
+ if (contains(block)) {
+ result = block;
+ return true;
+ }
+ block = oversizeBlockFor(ptr);
result = block;
- return !m_toSpaceFilter.ruleOut(reinterpret_cast<Bits>(block)) && m_toSpaceSet.contains(block);
+ return contains(block);
}
inline void CopiedSpace::pin(CopiedBlock* block)
@@ -53,7 +63,7 @@
m_fromSpace = m_toSpace;
m_toSpace = temp;
- m_toSpaceFilter.reset();
+ m_blockFilter.reset();
m_allocator.startedCopying();
ASSERT(!m_inCopyingPhase);
@@ -98,8 +108,8 @@
return false;
m_toSpace->push(block);
- m_toSpaceFilter.add(reinterpret_cast<Bits>(block));
- m_toSpaceSet.add(block);
+ m_blockFilter.add(reinterpret_cast<Bits>(block));
+ m_blockSet.add(block);
m_allocator.resetCurrentBlock(block);
return true;
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
