[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:36:52 UTC 2011


The following commit has been merged in the webkit-1.3 branch:
commit 86f2195180d0b74d779fa520957a3f3ce791750d
Author: ggaren at apple.com <ggaren at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Sat Jan 22 04:27:18 2011 +0000

    2011-01-21  Geoffrey Garen  <ggaren at apple.com>
    
            Reviewed by Maciej Stachowiak.
    
            Cleaned up some conservative marking code.
            https://bugs.webkit.org/show_bug.cgi?id=52946
    
            SunSpider reports no change.
    
            * interpreter/RegisterFile.h: No need for a special marking function,
            since we already expose a start() and end().
    
            * runtime/Heap.cpp:
            (JSC::Heap::registerFile):
            (JSC::Heap::markRoots):
            * runtime/Heap.h:
            (JSC::Heap::contains): Migrated markConservatively() to the machine stack
            marker class. Now, Heap just provides a contains() function, which the
            machine stack marker uses for checking whether a pointer points into the heap.
    
            * runtime/MachineStackMarker.cpp:
            (JSC::MachineStackMarker::markCurrentThreadConservativelyInternal):
            (JSC::MachineStackMarker::markOtherThreadConservatively):
            (JSC::isPointerAligned):
            (JSC::MachineStackMarker::markConservatively):
            * runtime/MachineStackMarker.h: Move the conservative marking code here.
    
            * runtime/MarkStack.h:
            (JSC::ConservativeSet::add):
            (JSC::ConservativeSet::mark): Changed to using a vector instead of hash
            set. Vector seems to be a bit faster, and it generates smaller code.
    
            * runtime/MarkedSpace.cpp:
            (JSC::MarkedSpace::containsSlowCase):
            * runtime/MarkedSpace.h:
            (JSC::MarkedSpace::isCellAligned):
            (JSC::MarkedSpace::isPossibleCell):
            (JSC::MarkedSpace::contains): Kept the code for determining whether a
            pointer pointed into marked space, and moved the code for marking
            a set of conservative pointers into the machine stack marker.
    
            * wtf/HashSet.h:
            (WTF::::add): Added two missing inlines that I noticed while testing
            vector vs hash set.
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76425 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 6115bde..40bf8e8 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,48 @@
+2011-01-21  Geoffrey Garen  <ggaren at apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        Cleaned up some conservative marking code.
+        https://bugs.webkit.org/show_bug.cgi?id=52946
+        
+        SunSpider reports no change.
+
+        * interpreter/RegisterFile.h: No need for a special marking function,
+        since we already expose a start() and end().
+
+        * runtime/Heap.cpp:
+        (JSC::Heap::registerFile):
+        (JSC::Heap::markRoots):
+        * runtime/Heap.h:
+        (JSC::Heap::contains): Migrated markConservatively() to the machine stack
+        marker class. Now, Heap just provides a contains() function, which the
+        machine stack marker uses for checking whether a pointer points into the heap.
+
+        * runtime/MachineStackMarker.cpp:
+        (JSC::MachineStackMarker::markCurrentThreadConservativelyInternal):
+        (JSC::MachineStackMarker::markOtherThreadConservatively):
+        (JSC::isPointerAligned):
+        (JSC::MachineStackMarker::markConservatively):
+        * runtime/MachineStackMarker.h: Move the conservative marking code here.
+
+        * runtime/MarkStack.h:
+        (JSC::ConservativeSet::add):
+        (JSC::ConservativeSet::mark): Changed to using a vector instead of hash
+        set. Vector seems to be a bit faster, and it generates smaller code.
+
+        * runtime/MarkedSpace.cpp:
+        (JSC::MarkedSpace::containsSlowCase):
+        * runtime/MarkedSpace.h:
+        (JSC::MarkedSpace::isCellAligned):
+        (JSC::MarkedSpace::isPossibleCell):
+        (JSC::MarkedSpace::contains): Kept the code for determining whether a
+        pointer pointed into marked space, and moved the code for marking
+        a set of conservative pointers into the machine stack marker.
+
+        * wtf/HashSet.h:
+        (WTF::::add): Added two missing inlines that I noticed while testing
+        vector vs hash set.
+
 2011-01-21  Mark Rowe  <mrowe at apple.com>
 
         Reviewed by Sam Weinig.
diff --git a/Source/JavaScriptCore/interpreter/RegisterFile.h b/Source/JavaScriptCore/interpreter/RegisterFile.h
index 9dfc432..e9c6df1 100644
--- a/Source/JavaScriptCore/interpreter/RegisterFile.h
+++ b/Source/JavaScriptCore/interpreter/RegisterFile.h
@@ -132,8 +132,6 @@ namespace JSC {
 
         Register* lastGlobal() const { return m_start - m_numGlobals; }
         
-        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 3966324..89f954b 100644
--- a/Source/JavaScriptCore/runtime/Heap.cpp
+++ b/Source/JavaScriptCore/runtime/Heap.cpp
@@ -147,11 +147,6 @@ void* Heap::allocate(size_t s)
     return result;
 }
 
-void Heap::markConservatively(ConservativeSet& conservativeSet, void* start, void* end)
-{
-    m_markedSpace.markConservatively(conservativeSet, start, end);
-}
-
 void Heap::updateWeakGCHandles()
 {
     for (unsigned i = 0; i < m_weakGCHandlePools.size(); ++i)
@@ -237,7 +232,12 @@ void Heap::markTempSortVectors(MarkStack& markStack)
         }
     }
 }
-    
+
+inline RegisterFile& Heap::registerFile()
+{
+    return m_globalData->interpreter->registerFile();
+}
+
 void Heap::markRoots()
 {
 #ifndef NDEBUG
@@ -258,7 +258,7 @@ void Heap::markRoots()
     // determine whether a reference is valid.
     ConservativeSet conservativeSet;
     m_machineStackMarker.markMachineStackConservatively(conservativeSet);
-    m_globalData->interpreter->registerFile().markCallFrames(conservativeSet, this);
+    m_machineStackMarker.markConservatively(conservativeSet, registerFile().start(), registerFile().end());
 
     // Reset mark bits.
     m_markedSpace.clearMarkBits();
diff --git a/Source/JavaScriptCore/runtime/Heap.h b/Source/JavaScriptCore/runtime/Heap.h
index f7f4deb..db05d81 100644
--- a/Source/JavaScriptCore/runtime/Heap.h
+++ b/Source/JavaScriptCore/runtime/Heap.h
@@ -31,15 +31,16 @@
 
 namespace JSC {
 
-    class JSValue;
-    class UString;
     class GCActivityCallback;
     class JSCell;
     class JSGlobalData;
     class JSValue;
+    class JSValue;
     class LiveObjectIterator;
-    class MarkedArgumentBuffer;
     class MarkStack;
+    class MarkedArgumentBuffer;
+    class RegisterFile;
+    class UString;
     class WeakGCHandlePool;
 
     typedef std::pair<JSValue, UString> ValueStringPair;
@@ -85,11 +86,11 @@ namespace JSC {
         static bool isCellMarked(const JSCell*);
         static bool checkMarkCell(const JSCell*);
         static void markCell(JSCell*);
+        
+        bool contains(void*);
 
         WeakGCHandle* addWeakGCHandle(JSCell*);
 
-        void markConservatively(ConservativeSet&, void* start, void* end);
-
         void pushTempSortVector(WTF::Vector<ValueStringPair>*);
         void popTempSortVector(WTF::Vector<ValueStringPair>*);        
 
@@ -118,6 +119,8 @@ namespace JSC {
         void updateWeakGCHandles();
         WeakGCHandlePool* weakGCHandlePool(size_t index);
 
+        RegisterFile& registerFile();
+
         MarkedSpace m_markedSpace;
         OperationInProgress m_operationInProgress;
 
@@ -152,6 +155,11 @@ namespace JSC {
         MarkedSpace::markCell(cell);
     }
 
+    inline bool Heap::contains(void* p)
+    {
+        return m_markedSpace.contains(p);
+    }
+
     inline void Heap::reportExtraMemoryCost(size_t cost)
     {
         if (cost > minExtraCost) 
diff --git a/Source/JavaScriptCore/runtime/MachineStackMarker.cpp b/Source/JavaScriptCore/runtime/MachineStackMarker.cpp
index e52f402..4e3408d 100644
--- a/Source/JavaScriptCore/runtime/MachineStackMarker.cpp
+++ b/Source/JavaScriptCore/runtime/MachineStackMarker.cpp
@@ -196,7 +196,7 @@ void MachineStackMarker::unregisterThread()
 
 void NEVER_INLINE MachineStackMarker::markCurrentThreadConservativelyInternal(ConservativeSet& conservativeSet)
 {
-    m_heap->markConservatively(conservativeSet, m_heap->globalData()->stack().current(), m_heap->globalData()->stack().origin());
+    markConservatively(conservativeSet, m_heap->globalData()->stack().current(), m_heap->globalData()->stack().origin());
 }
 
 #if COMPILER(GCC)
@@ -358,10 +358,10 @@ void MachineStackMarker::markOtherThreadConservatively(ConservativeSet& conserva
     size_t regSize = getPlatformThreadRegisters(thread->platformThread, regs);
 
     // mark the thread's registers
-    m_heap->markConservatively(conservativeSet, static_cast<void*>(&regs), static_cast<void*>(reinterpret_cast<char*>(&regs) + regSize));
+    markConservatively(conservativeSet, static_cast<void*>(&regs), static_cast<void*>(reinterpret_cast<char*>(&regs) + regSize));
 
     void* stackPointer = otherThreadStackPointer(regs);
-    m_heap->markConservatively(conservativeSet, stackPointer, thread->stackBase);
+    markConservatively(conservativeSet, stackPointer, thread->stackBase);
 
     resumeThread(thread->platformThread);
 }
@@ -397,4 +397,36 @@ void MachineStackMarker::markMachineStackConservatively(ConservativeSet& conserv
 #endif
 }
 
+inline bool isPointerAligned(void* p)
+{
+    return (((intptr_t)(p) & (sizeof(char*) - 1)) == 0);
+}
+
+void MachineStackMarker::markConservatively(ConservativeSet& conservativeSet, void* start, void* end)
+{
+#if OS(WINCE)
+    if (start > end) {
+        void* tmp = start;
+        start = end;
+        end = tmp;
+    }
+#else
+    ASSERT(start <= end);
+#endif
+
+    ASSERT((static_cast<char*>(end) - static_cast<char*>(start)) < 0x1000000);
+    ASSERT(isPointerAligned(start));
+    ASSERT(isPointerAligned(end));
+
+    char** p = static_cast<char**>(start);
+    char** e = static_cast<char**>(end);
+
+    while (p != e) {
+        char* x = *p++;
+        if (!m_heap->contains(x))
+            continue;
+        conservativeSet.add(reinterpret_cast<JSCell*>(x));
+    }
+}
+
 } // namespace JSC
diff --git a/Source/JavaScriptCore/runtime/MachineStackMarker.h b/Source/JavaScriptCore/runtime/MachineStackMarker.h
index 8afdb46..a1f3173 100644
--- a/Source/JavaScriptCore/runtime/MachineStackMarker.h
+++ b/Source/JavaScriptCore/runtime/MachineStackMarker.h
@@ -41,6 +41,7 @@ namespace JSC {
         ~MachineStackMarker();
 
         void markMachineStackConservatively(ConservativeSet&);
+        void markConservatively(ConservativeSet&, void* start, void* end);
 
 #if ENABLE(JSC_MULTIPLE_THREADS)
         void makeUsableFromMultipleThreads();
diff --git a/Source/JavaScriptCore/runtime/MarkStack.h b/Source/JavaScriptCore/runtime/MarkStack.h
index 7946e65..58dfa4c 100644
--- a/Source/JavaScriptCore/runtime/MarkStack.h
+++ b/Source/JavaScriptCore/runtime/MarkStack.h
@@ -27,7 +27,7 @@
 #define MarkStack_h
 
 #include "JSValue.h"
-#include <wtf/HashSet.h>
+#include <wtf/Vector.h>
 #include <wtf/Noncopyable.h>
 #include <wtf/OSAllocator.h>
 
@@ -190,16 +190,15 @@ namespace JSC {
     
     class ConservativeSet {
     public:
-        void add(JSCell* cell) { m_set.add(cell); }
+        void add(JSCell* cell) { m_vector.append(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);
+            for (size_t i = 0; i < m_vector.size(); ++i)
+                markStack.append(m_vector[i]);
         }
 
     private:
-        HashSet<JSCell*> m_set;
+        Vector<JSCell*, 64> m_vector;
     };
 }
 
diff --git a/Source/JavaScriptCore/runtime/MarkedSpace.cpp b/Source/JavaScriptCore/runtime/MarkedSpace.cpp
index 036c8f0..c020617 100644
--- a/Source/JavaScriptCore/runtime/MarkedSpace.cpp
+++ b/Source/JavaScriptCore/runtime/MarkedSpace.cpp
@@ -200,82 +200,37 @@ void MarkedSpace::shrinkBlocks(size_t neededBlocks)
         m_heap.collectorBlock(i)->marked.set(HeapConstants::cellsPerBlock - 1);
 }
 
-inline bool isPointerAligned(void* p)
+bool MarkedSpace::containsSlowCase(void* x)
 {
-    return (((intptr_t)(p) & (sizeof(char*) - 1)) == 0);
-}
-
-// Cell size needs to be a power of two for isPossibleCell to be valid.
-COMPILE_ASSERT(sizeof(CollectorCell) % 2 == 0, Collector_cell_size_is_power_of_two);
-
-static inline bool isCellAligned(void *p)
-{
-    return (((intptr_t)(p) & CELL_MASK) == 0);
-}
-
-static inline bool isPossibleCell(void* p)
-{
-    return isCellAligned(p) && p;
-}
-
-void MarkedSpace::markConservatively(ConservativeSet& conservativeSet, void* start, void* end)
-{
-#if OS(WINCE)
-    if (start > end) {
-        void* tmp = start;
-        start = end;
-        end = tmp;
-    }
-#else
-    ASSERT(start <= end);
-#endif
-
-    ASSERT((static_cast<char*>(end) - static_cast<char*>(start)) < 0x1000000);
-    ASSERT(isPointerAligned(start));
-    ASSERT(isPointerAligned(end));
-
-    char** p = static_cast<char**>(start);
-    char** e = static_cast<char**>(end);
-
-    while (p != e) {
-        char* x = *p++;
-        if (isPossibleCell(x)) {
-            uintptr_t xAsBits = reinterpret_cast<uintptr_t>(x);
-            xAsBits &= CELL_ALIGN_MASK;
-
-            uintptr_t offset = xAsBits & BLOCK_OFFSET_MASK;
-            const size_t lastCellOffset = sizeof(CollectorCell) * (CELLS_PER_BLOCK - 1);
-            if (offset > lastCellOffset)
-                continue;
-
-            CollectorBlock* blockAddr = reinterpret_cast<CollectorBlock*>(xAsBits - offset);
-            size_t usedBlocks = m_heap.usedBlocks;
-            for (size_t block = 0; block < usedBlocks; block++) {
-                if (m_heap.collectorBlock(block) != blockAddr)
-                    continue;
-
-                // 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;
-                }
-            }
-        }
+    uintptr_t xAsBits = reinterpret_cast<uintptr_t>(x);
+    xAsBits &= CELL_ALIGN_MASK;
+
+    uintptr_t offset = xAsBits & BLOCK_OFFSET_MASK;
+    const size_t lastCellOffset = sizeof(CollectorCell) * (CELLS_PER_BLOCK - 1);
+    if (offset > lastCellOffset)
+        return false;
+
+    CollectorBlock* blockAddr = reinterpret_cast<CollectorBlock*>(xAsBits - offset);
+    size_t usedBlocks = m_heap.usedBlocks;
+    for (size_t block = 0; block < usedBlocks; block++) {
+        if (m_heap.collectorBlock(block) != blockAddr)
+            continue;
+
+        // 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)
+            return true;
+        
+        size_t cellOffset = offset / CELL_SIZE;
+        
+        if (block == m_heap.nextBlock && cellOffset < m_heap.nextCell)
+            return true;
+        
+        return blockAddr->marked.get(cellOffset);
     }
+    
+    return false;
 }
 
 void MarkedSpace::clearMarkBits()
diff --git a/Source/JavaScriptCore/runtime/MarkedSpace.h b/Source/JavaScriptCore/runtime/MarkedSpace.h
index af312b5..6357664 100644
--- a/Source/JavaScriptCore/runtime/MarkedSpace.h
+++ b/Source/JavaScriptCore/runtime/MarkedSpace.h
@@ -85,10 +85,11 @@ namespace JSC {
 
         WeakGCHandle* addWeakGCHandle(JSCell*);
 
-        void markConservatively(ConservativeSet&, void* start, void* end);
+        bool contains(void*);
+        bool containsSlowCase(void*);
+        bool isCellAligned(void*);
+        bool isPossibleCell(void*);
 
-        static bool isNumber(JSCell*);
-        
         LiveObjectIterator primaryHeapBegin();
         LiveObjectIterator primaryHeapEnd();
 
@@ -217,6 +218,27 @@ namespace JSC {
         cellBlock(cell)->marked.set(cellOffset(cell));
     }
 
+    // Cell size needs to be a power of two for isPossibleCell to be valid.
+    COMPILE_ASSERT(sizeof(CollectorCell) % 2 == 0, Collector_cell_size_is_power_of_two);
+
+    inline bool MarkedSpace::isCellAligned(void *p)
+    {
+        return (((intptr_t)(p) & CELL_MASK) == 0);
+    }
+
+    inline bool MarkedSpace::isPossibleCell(void* p)
+    {
+        return isCellAligned(p) && p;
+    }
+
+    inline bool MarkedSpace::contains(void* x)
+    {
+        if (!isPossibleCell(x))
+            return false;
+            
+        return containsSlowCase(x);
+    }
+
 } // namespace JSC
 
 #endif // MarkedSpace_h
diff --git a/Source/JavaScriptCore/wtf/HashSet.h b/Source/JavaScriptCore/wtf/HashSet.h
index be6b93d..82245f3 100644
--- a/Source/JavaScriptCore/wtf/HashSet.h
+++ b/Source/JavaScriptCore/wtf/HashSet.h
@@ -175,14 +175,14 @@ namespace WTF {
     }
 
     template<typename T, typename U, typename V>
-    pair<typename HashSet<T, U, V>::iterator, bool> HashSet<T, U, V>::add(const ValueType& value)
+    inline pair<typename HashSet<T, U, V>::iterator, bool> HashSet<T, U, V>::add(const ValueType& value)
     {
         return m_impl.add(value);
     }
 
     template<typename Value, typename HashFunctions, typename Traits>
     template<typename T, typename HashTranslator>
-    pair<typename HashSet<Value, HashFunctions, Traits>::iterator, bool>
+    inline pair<typename HashSet<Value, HashFunctions, Traits>::iterator, bool>
     HashSet<Value, HashFunctions, Traits>::add(const T& value)
     {
         typedef HashSetTranslatorAdapter<ValueType, ValueTraits, T, HashTranslator> Adapter;

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list