[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198
ggaren at apple.com
ggaren at apple.com
Sun Feb 20 23:31:38 UTC 2011
The following commit has been merged in the webkit-1.3 branch:
commit 7c9ff4d47b2cdfdf337f9dc519d4861fe219a686
Author: ggaren at apple.com <ggaren at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Jan 21 04:07:38 2011 +0000
2011-01-20 Geoffrey Garen <ggaren at apple.com>
Reviewed by Oliver Hunt.
When marking conservatively, guard against reviving dead objects.
https://bugs.webkit.org/show_bug.cgi?id=52840
SunSpider and v8 say no change.
* interpreter/RegisterFile.h:
(JSC::RegisterFile::markCallFrames): Updated to use the ConservativeSet API.
* runtime/Heap.cpp:
(JSC::Heap::recordExtraCost): No need to guard against conservative
marking reviving dead objects anymore, since the conservative marking
mechanism guards against this now.
(JSC::Heap::markConservatively):
(JSC::Heap::markProtectedObjects):
(JSC::Heap::markTempSortVectors): Don't drain the mark stack inside a
marking function. We want to establish a separation of concerns between
visiting roots and draining the mark stack.
(JSC::Heap::markRoots): Gather the set of conservative references before
clearning mark bits, because conservative marking now uses the mark bits
to determine if a reference is valid, and avoid reviving dead objects.
(JSC::Heap::collectAllGarbage): No need to guard against conservative
marking reviving dead objects anymore, since the conservative marking
mechanism guards against this now.
* runtime/Heap.h: Updated to use the ConservativeSet API.
* runtime/MachineStackMarker.cpp:
(JSC::MachineStackMarker::markCurrentThreadConservativelyInternal):
(JSC::MachineStackMarker::markCurrentThreadConservatively):
(JSC::MachineStackMarker::markOtherThreadConservatively):
(JSC::MachineStackMarker::markMachineStackConservatively):
* runtime/MachineStackMarker.h: Ditto.
* runtime/MarkStack.h:
(JSC::ConservativeSet::add):
(JSC::ConservativeSet::mark): Added ConservativeSet, for gathering the
set of conservative references. This is different from MarkStack, since
we don't mark the set until it is completely gathered.
* runtime/MarkedSpace.cpp:
(JSC::MarkedSpace::freeBlock):
(JSC::MarkedSpace::resizeBlocks):
(JSC::MarkedSpace::markConservatively):
* runtime/MarkedSpace.h: When marking conservatively, guard against
reviving dead objects.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76331 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 0ea36db..23696c6 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,56 @@
+2011-01-20 Geoffrey Garen <ggaren at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ When marking conservatively, guard against reviving dead objects.
+ https://bugs.webkit.org/show_bug.cgi?id=52840
+
+ SunSpider and v8 say no change.
+
+ * interpreter/RegisterFile.h:
+ (JSC::RegisterFile::markCallFrames): Updated to use the ConservativeSet API.
+
+ * runtime/Heap.cpp:
+ (JSC::Heap::recordExtraCost): No need to guard against conservative
+ marking reviving dead objects anymore, since the conservative marking
+ mechanism guards against this now.
+
+ (JSC::Heap::markConservatively):
+ (JSC::Heap::markProtectedObjects):
+ (JSC::Heap::markTempSortVectors): Don't drain the mark stack inside a
+ marking function. We want to establish a separation of concerns between
+ visiting roots and draining the mark stack.
+
+ (JSC::Heap::markRoots): Gather the set of conservative references before
+ clearning mark bits, because conservative marking now uses the mark bits
+ to determine if a reference is valid, and avoid reviving dead objects.
+
+ (JSC::Heap::collectAllGarbage): No need to guard against conservative
+ marking reviving dead objects anymore, since the conservative marking
+ mechanism guards against this now.
+
+ * runtime/Heap.h: Updated to use the ConservativeSet API.
+
+ * runtime/MachineStackMarker.cpp:
+ (JSC::MachineStackMarker::markCurrentThreadConservativelyInternal):
+ (JSC::MachineStackMarker::markCurrentThreadConservatively):
+ (JSC::MachineStackMarker::markOtherThreadConservatively):
+ (JSC::MachineStackMarker::markMachineStackConservatively):
+ * runtime/MachineStackMarker.h: Ditto.
+
+ * runtime/MarkStack.h:
+ (JSC::ConservativeSet::add):
+ (JSC::ConservativeSet::mark): Added ConservativeSet, for gathering the
+ set of conservative references. This is different from MarkStack, since
+ we don't mark the set until it is completely gathered.
+
+ * runtime/MarkedSpace.cpp:
+ (JSC::MarkedSpace::freeBlock):
+ (JSC::MarkedSpace::resizeBlocks):
+ (JSC::MarkedSpace::markConservatively):
+ * runtime/MarkedSpace.h: When marking conservatively, guard against
+ reviving dead objects.
+
2011-01-20 Siddharth Mathur <siddharth.mathur at nokia.com>
Reviewed by Geoffrey Garen.
diff --git a/Source/JavaScriptCore/interpreter/RegisterFile.h b/Source/JavaScriptCore/interpreter/RegisterFile.h
index 3498e90..9dfc432 100644
--- a/Source/JavaScriptCore/interpreter/RegisterFile.h
+++ b/Source/JavaScriptCore/interpreter/RegisterFile.h
@@ -132,7 +132,7 @@ namespace JSC {
Register* lastGlobal() const { return m_start - m_numGlobals; }
- void markCallFrames(MarkStack& markStack, Heap* heap) { heap->markConservatively(markStack, m_start, m_end); }
+ void markCallFrames(ConservativeSet& conservativeSet, Heap* heap) { heap->markConservatively(conservativeSet, m_start, m_end); }
static size_t committedByteCount();
static void initializeThreading();
diff --git a/Source/JavaScriptCore/runtime/Heap.cpp b/Source/JavaScriptCore/runtime/Heap.cpp
index a224ee0..e2cc329 100644
--- a/Source/JavaScriptCore/runtime/Heap.cpp
+++ b/Source/JavaScriptCore/runtime/Heap.cpp
@@ -93,12 +93,6 @@ void Heap::recordExtraCost(size_t cost)
if (m_extraCost > maxExtraCost && m_extraCost > m_markedSpace.size() / 2) {
JAVASCRIPTCORE_GC_BEGIN();
- // If the last iteration through the heap deallocated blocks, we need
- // to clean up remaining garbage before marking. Otherwise, the conservative
- // marking mechanism might follow a pointer to unmapped memory.
- if (m_markedSpace.didShrink())
- m_markedSpace.sweep();
-
markRoots();
JAVASCRIPTCORE_GC_MARKED();
@@ -152,9 +146,9 @@ void* Heap::allocate(size_t s)
return result;
}
-void Heap::markConservatively(MarkStack& markStack, void* start, void* end)
+void Heap::markConservatively(ConservativeSet& conservativeSet, void* start, void* end)
{
- m_markedSpace.markConservatively(markStack, start, end);
+ m_markedSpace.markConservatively(conservativeSet, start, end);
}
void Heap::updateWeakGCHandles()
@@ -212,10 +206,8 @@ bool Heap::unprotect(JSValue k)
void Heap::markProtectedObjects(MarkStack& markStack)
{
ProtectCountSet::iterator end = m_protectedValues.end();
- for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it) {
+ for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it)
markStack.append(it->first);
- markStack.drain();
- }
}
void Heap::pushTempSortVector(Vector<ValueStringPair>* tempVector)
@@ -238,10 +230,10 @@ void Heap::markTempSortVectors(MarkStack& markStack)
Vector<ValueStringPair>* tempSortingVector = *it;
Vector<ValueStringPair>::iterator vectorEnd = tempSortingVector->end();
- for (Vector<ValueStringPair>::iterator vectorIt = tempSortingVector->begin(); vectorIt != vectorEnd; ++vectorIt)
+ for (Vector<ValueStringPair>::iterator vectorIt = tempSortingVector->begin(); vectorIt != vectorEnd; ++vectorIt) {
if (vectorIt->first)
markStack.append(vectorIt->first);
- markStack.drain();
+ }
}
}
@@ -260,20 +252,27 @@ void Heap::markRoots()
m_operationInProgress = Collection;
- MarkStack& markStack = m_globalData->markStack;
+ // We gather the conservative set before clearing mark bits, because
+ // conservative gathering uses the mark bits from our last mark pass to
+ // determine whether a reference is valid.
+ ConservativeSet conservativeSet;
+ m_machineStackMarker.markMachineStackConservatively(conservativeSet);
+ m_globalData->interpreter->registerFile().markCallFrames(conservativeSet, this);
// Reset mark bits.
m_markedSpace.clearMarkBits();
- // Mark stack roots.
- m_machineStackMarker.markMachineStackConservatively(markStack);
- m_globalData->interpreter->registerFile().markCallFrames(markStack, this);
+ MarkStack& markStack = m_globalData->markStack;
+ conservativeSet.mark(markStack);
+ markStack.drain();
// Mark explicitly registered roots.
markProtectedObjects(markStack);
+ markStack.drain();
// Mark temporary vector for Array sorting
markTempSortVectors(markStack);
+ markStack.drain();
// Mark misc. other roots.
if (m_markListSet && m_markListSet->size())
@@ -282,6 +281,7 @@ void Heap::markRoots()
markStack.append(m_globalData->exception);
if (m_globalData->firstStringifierToMark)
JSONObject::markStringifiers(markStack, m_globalData->firstStringifierToMark);
+ markStack.drain();
// Mark the small strings cache last, since it will clear itself if nothing
// else has marked it.
@@ -392,12 +392,6 @@ void Heap::collectAllGarbage()
ASSERT(globalData()->identifierTable == wtfThreadData().currentIdentifierTable());
JAVASCRIPTCORE_GC_BEGIN();
- // If the last iteration through the heap deallocated blocks, we need
- // to clean up remaining garbage before marking. Otherwise, the conservative
- // marking mechanism might follow a pointer to unmapped memory.
- if (m_markedSpace.didShrink())
- m_markedSpace.sweep();
-
markRoots();
JAVASCRIPTCORE_GC_MARKED();
diff --git a/Source/JavaScriptCore/runtime/Heap.h b/Source/JavaScriptCore/runtime/Heap.h
index 3e1d886..f1eba2b 100644
--- a/Source/JavaScriptCore/runtime/Heap.h
+++ b/Source/JavaScriptCore/runtime/Heap.h
@@ -87,7 +87,7 @@ namespace JSC {
WeakGCHandle* addWeakGCHandle(JSCell*);
- void markConservatively(MarkStack&, void* start, void* end);
+ void markConservatively(ConservativeSet&, void* start, void* end);
void pushTempSortVector(WTF::Vector<ValueStringPair>*);
void popTempSortVector(WTF::Vector<ValueStringPair>*);
diff --git a/Source/JavaScriptCore/runtime/MachineStackMarker.cpp b/Source/JavaScriptCore/runtime/MachineStackMarker.cpp
index b4a1936..e52f402 100644
--- a/Source/JavaScriptCore/runtime/MachineStackMarker.cpp
+++ b/Source/JavaScriptCore/runtime/MachineStackMarker.cpp
@@ -194,10 +194,9 @@ void MachineStackMarker::unregisterThread()
#endif
-void NEVER_INLINE MachineStackMarker::markCurrentThreadConservativelyInternal(MarkStack& markStack)
+void NEVER_INLINE MachineStackMarker::markCurrentThreadConservativelyInternal(ConservativeSet& conservativeSet)
{
- m_heap->markConservatively(markStack, m_heap->globalData()->stack().current(), m_heap->globalData()->stack().origin());
- markStack.drain();
+ m_heap->markConservatively(conservativeSet, m_heap->globalData()->stack().current(), m_heap->globalData()->stack().origin());
}
#if COMPILER(GCC)
@@ -206,7 +205,7 @@ void NEVER_INLINE MachineStackMarker::markCurrentThreadConservativelyInternal(Ma
#define REGISTER_BUFFER_ALIGNMENT
#endif
-void MachineStackMarker::markCurrentThreadConservatively(MarkStack& markStack)
+void MachineStackMarker::markCurrentThreadConservatively(ConservativeSet& conservativeSet)
{
// setjmp forces volatile registers onto the stack
jmp_buf registers REGISTER_BUFFER_ALIGNMENT;
@@ -219,7 +218,7 @@ void MachineStackMarker::markCurrentThreadConservatively(MarkStack& markStack)
#pragma warning(pop)
#endif
- markCurrentThreadConservativelyInternal(markStack);
+ markCurrentThreadConservativelyInternal(conservativeSet);
}
#if ENABLE(JSC_MULTIPLE_THREADS)
@@ -351,7 +350,7 @@ static inline void* otherThreadStackPointer(const PlatformThreadRegisters& regs)
#endif
}
-void MachineStackMarker::markOtherThreadConservatively(MarkStack& markStack, Thread* thread)
+void MachineStackMarker::markOtherThreadConservatively(ConservativeSet& conservativeSet, Thread* thread)
{
suspendThread(thread->platformThread);
@@ -359,21 +358,19 @@ void MachineStackMarker::markOtherThreadConservatively(MarkStack& markStack, Thr
size_t regSize = getPlatformThreadRegisters(thread->platformThread, regs);
// mark the thread's registers
- m_heap->markConservatively(markStack, static_cast<void*>(®s), static_cast<void*>(reinterpret_cast<char*>(®s) + regSize));
- markStack.drain();
+ m_heap->markConservatively(conservativeSet, static_cast<void*>(®s), static_cast<void*>(reinterpret_cast<char*>(®s) + regSize));
void* stackPointer = otherThreadStackPointer(regs);
- m_heap->markConservatively(markStack, stackPointer, thread->stackBase);
- markStack.drain();
+ m_heap->markConservatively(conservativeSet, stackPointer, thread->stackBase);
resumeThread(thread->platformThread);
}
#endif
-void MachineStackMarker::markMachineStackConservatively(MarkStack& markStack)
+void MachineStackMarker::markMachineStackConservatively(ConservativeSet& conservativeSet)
{
- markCurrentThreadConservatively(markStack);
+ markCurrentThreadConservatively(conservativeSet);
#if ENABLE(JSC_MULTIPLE_THREADS)
@@ -391,7 +388,7 @@ void MachineStackMarker::markMachineStackConservatively(MarkStack& markStack)
// and since this is a shared heap, they are real locks.
for (Thread* thread = m_registeredThreads; thread; thread = thread->next) {
if (!pthread_equal(thread->posixThread, pthread_self()))
- markOtherThreadConservatively(markStack, thread);
+ markOtherThreadConservatively(conservativeSet, thread);
}
#ifndef NDEBUG
fastMallocAllow();
diff --git a/Source/JavaScriptCore/runtime/MachineStackMarker.h b/Source/JavaScriptCore/runtime/MachineStackMarker.h
index b186834..8afdb46 100644
--- a/Source/JavaScriptCore/runtime/MachineStackMarker.h
+++ b/Source/JavaScriptCore/runtime/MachineStackMarker.h
@@ -32,7 +32,7 @@
namespace JSC {
class Heap;
- class MarkStack;
+ class ConservativeSet;
class MachineStackMarker {
WTF_MAKE_NONCOPYABLE(MachineStackMarker);
@@ -40,7 +40,7 @@ namespace JSC {
MachineStackMarker(Heap*);
~MachineStackMarker();
- void markMachineStackConservatively(MarkStack&);
+ void markMachineStackConservatively(ConservativeSet&);
#if ENABLE(JSC_MULTIPLE_THREADS)
void makeUsableFromMultipleThreads();
@@ -48,8 +48,8 @@ namespace JSC {
#endif
private:
- void markCurrentThreadConservatively(MarkStack&);
- void markCurrentThreadConservativelyInternal(MarkStack&);
+ void markCurrentThreadConservatively(ConservativeSet&);
+ void markCurrentThreadConservativelyInternal(ConservativeSet&);
#if ENABLE(JSC_MULTIPLE_THREADS)
class Thread;
@@ -57,7 +57,7 @@ namespace JSC {
static void unregisterThread(void*);
void unregisterThread();
- void markOtherThreadConservatively(MarkStack&, Thread*);
+ void markOtherThreadConservatively(ConservativeSet&, Thread*);
#endif
Heap* m_heap;
diff --git a/Source/JavaScriptCore/runtime/MarkStack.h b/Source/JavaScriptCore/runtime/MarkStack.h
index f54cfa6..7946e65 100644
--- a/Source/JavaScriptCore/runtime/MarkStack.h
+++ b/Source/JavaScriptCore/runtime/MarkStack.h
@@ -27,6 +27,7 @@
#define MarkStack_h
#include "JSValue.h"
+#include <wtf/HashSet.h>
#include <wtf/Noncopyable.h>
#include <wtf/OSAllocator.h>
@@ -186,6 +187,20 @@ namespace JSC {
bool m_isDraining;
#endif
};
+
+ class ConservativeSet {
+ public:
+ void add(JSCell* cell) { m_set.add(cell); }
+ void mark(MarkStack& markStack)
+ {
+ HashSet<JSCell*>::iterator end = m_set.end();
+ for (HashSet<JSCell*>::iterator it = m_set.begin(); it != end; ++it)
+ markStack.append(*it);
+ }
+
+ private:
+ HashSet<JSCell*> m_set;
+ };
}
#endif
diff --git a/Source/JavaScriptCore/runtime/MarkedSpace.cpp b/Source/JavaScriptCore/runtime/MarkedSpace.cpp
index 4bc3c18..036c8f0 100644
--- a/Source/JavaScriptCore/runtime/MarkedSpace.cpp
+++ b/Source/JavaScriptCore/runtime/MarkedSpace.cpp
@@ -108,8 +108,6 @@ NEVER_INLINE CollectorBlock* MarkedSpace::allocateBlock()
NEVER_INLINE void MarkedSpace::freeBlock(size_t block)
{
- m_heap.didShrink = true;
-
ObjectIterator it(m_heap, block);
ObjectIterator end(m_heap, block + 1);
for ( ; it != end; ++it)
@@ -162,8 +160,6 @@ void* MarkedSpace::allocate(size_t s)
void MarkedSpace::resizeBlocks()
{
- m_heap.didShrink = false;
-
size_t usedCellCount = markedCells();
size_t minCellCount = usedCellCount + max(ALLOCATIONS_PER_COLLECTION, usedCellCount);
size_t minBlockCount = (minCellCount + HeapConstants::cellsPerBlock - 1) / HeapConstants::cellsPerBlock;
@@ -222,7 +218,7 @@ static inline bool isPossibleCell(void* p)
return isCellAligned(p) && p;
}
-void MarkedSpace::markConservatively(MarkStack& markStack, void* start, void* end)
+void MarkedSpace::markConservatively(ConservativeSet& conservativeSet, void* start, void* end)
{
#if OS(WINCE)
if (start > end) {
@@ -244,7 +240,6 @@ void MarkedSpace::markConservatively(MarkStack& markStack, void* start, void* en
while (p != e) {
char* x = *p++;
if (isPossibleCell(x)) {
- size_t usedBlocks;
uintptr_t xAsBits = reinterpret_cast<uintptr_t>(x);
xAsBits &= CELL_ALIGN_MASK;
@@ -254,11 +249,30 @@ void MarkedSpace::markConservatively(MarkStack& markStack, void* start, void* en
continue;
CollectorBlock* blockAddr = reinterpret_cast<CollectorBlock*>(xAsBits - offset);
- usedBlocks = m_heap.usedBlocks;
+ size_t usedBlocks = m_heap.usedBlocks;
for (size_t block = 0; block < usedBlocks; block++) {
if (m_heap.collectorBlock(block) != blockAddr)
continue;
- markStack.append(reinterpret_cast<JSCell*>(xAsBits));
+
+ // x is a pointer into the heap. Now, verify that the cell it
+ // points to is live. (If the cell is dead, we must not mark it,
+ // since that would revive it in a zombie state.)
+ if (block < m_heap.nextBlock) {
+ conservativeSet.add(reinterpret_cast<JSCell*>(xAsBits));
+ break;
+ }
+
+ size_t cellOffset = offset / CELL_SIZE;
+
+ if (block == m_heap.nextBlock && cellOffset < m_heap.nextCell) {
+ conservativeSet.add(reinterpret_cast<JSCell*>(xAsBits));
+ break;
+ }
+
+ if (blockAddr->marked.get(cellOffset)) {
+ conservativeSet.add(reinterpret_cast<JSCell*>(xAsBits));
+ break;
+ }
}
}
}
diff --git a/Source/JavaScriptCore/runtime/MarkedSpace.h b/Source/JavaScriptCore/runtime/MarkedSpace.h
index 3b0c5e0..af312b5 100644
--- a/Source/JavaScriptCore/runtime/MarkedSpace.h
+++ b/Source/JavaScriptCore/runtime/MarkedSpace.h
@@ -55,8 +55,6 @@ namespace JSC {
size_t numBlocks;
size_t usedBlocks;
- bool didShrink;
-
CollectorBlock* collectorBlock(size_t index) const
{
return static_cast<CollectorBlock*>(blocks[index].base());
@@ -87,7 +85,7 @@ namespace JSC {
WeakGCHandle* addWeakGCHandle(JSCell*);
- void markConservatively(MarkStack&, void* start, void* end);
+ void markConservatively(ConservativeSet&, void* start, void* end);
static bool isNumber(JSCell*);
@@ -115,8 +113,6 @@ namespace JSC {
void markRoots();
- bool didShrink() { return m_heap.didShrink; }
-
private:
CollectorHeap m_heap;
JSGlobalData* m_globalData;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list