Changeset 81262 in webkit

Timestamp:

Mar 16, 2011 11:35:49 AM (13 years ago)

Author:

ggaren@apple.com

Message:

2011-03-16 Geoffrey Garen <​ggaren@apple.com>

Reviewed by Oliver Hunt.

Some conservative root gathering cleanup
​https://bugs.webkit.org/show_bug.cgi?id=56447

SunSpider says 0.5% - 1.8% faster.

• interpreter/RegisterFile.cpp: (JSC::RegisterFile::gatherConservativeRoots):
• interpreter/RegisterFile.h: New helper function for doing the conservative gathering of the register file. It's still conservative, since the register file may contain uninitialized values, but it's moving-safe, because it only visits values tagged as pointers, so there's no risk of mistaking an integer for a pointer and accidentally changing it.

• runtime/ConservativeSet.cpp: (JSC::ConservativeRoots::add):
• runtime/ConservativeSet.h: Added a single-value add function, used above.

• runtime/Heap.cpp: (JSC::Heap::markRoots): Separated machine stack conservative roots from register file conservative roots because machine stack roots must be pinned, but register file roots need not be pinned.

Adopted new interface for passing the current stack extent to the machine
stack root gathering routine. This allows us to exclude marking-related
data structures on the stack, and thus avoid double-marking the set of
machine roots.

• runtime/MachineStackMarker.cpp: (JSC::MachineThreads::gatherFromCurrentThread): (JSC::MachineThreads::gatherConservativeRoots):
• runtime/MachineStackMarker.h: Added new interface, described above.

• runtime/MarkedBlock.h: (JSC::MarkedBlock::firstAtom):
• wtf/StdLibExtras.h: (WTF::roundUpToMultipleOf): Moved roundUpToMultipleOf so it could be used by MachineStacks.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• interpreter/RegisterFile.cpp (modified) (2 diffs)
• interpreter/RegisterFile.h (modified) (1 diff)
• runtime/ConservativeSet.cpp (modified) (2 diffs)
• runtime/ConservativeSet.h (modified) (3 diffs)
• runtime/Heap.cpp (modified) (2 diffs)
• runtime/MachineStackMarker.cpp (modified) (6 diffs)
• runtime/MachineStackMarker.h (modified) (2 diffs)
• runtime/MarkStack.cpp (modified) (2 diffs)
• runtime/MarkStack.h (modified) (3 diffs)
• runtime/MarkedBlock.h (modified) (3 diffs)
• wtf/StdLibExtras.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-03-16  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        Some conservative root gathering cleanup
+        https://bugs.webkit.org/show_bug.cgi?id=56447
+        
+        SunSpider says 0.5% - 1.8% faster.
+
+        * interpreter/RegisterFile.cpp:
+        (JSC::RegisterFile::gatherConservativeRoots):
+        * interpreter/RegisterFile.h: New helper function for doing the
+        conservative gathering of the register file. It's still conservative,
+        since the register file may contain uninitialized values, but it's
+        moving-safe, because it only visits values tagged as pointers, so there's
+        no risk of mistaking an integer for a pointer and accidentally changing it.
+
+        * runtime/ConservativeSet.cpp:
+        (JSC::ConservativeRoots::add):
+        * runtime/ConservativeSet.h: Added a single-value add function, used above.
+
+        * runtime/Heap.cpp:
+        (JSC::Heap::markRoots): Separated machine stack conservative roots from
+        register file conservative roots because machine stack roots must be
+        pinned, but register file roots need not be pinned.
+        
+        Adopted new interface for passing the current stack extent to the machine
+        stack root gathering routine. This allows us to exclude marking-related
+        data structures on the stack, and thus avoid double-marking the set of
+        machine roots.
+
+        * runtime/MachineStackMarker.cpp:
+        (JSC::MachineThreads::gatherFromCurrentThread):
+        (JSC::MachineThreads::gatherConservativeRoots):
+        * runtime/MachineStackMarker.h: Added new interface, described above.
+
+        * runtime/MarkedBlock.h:
+        (JSC::MarkedBlock::firstAtom):
+        * wtf/StdLibExtras.h:
+        (WTF::roundUpToMultipleOf): Moved roundUpToMultipleOf so it could be used
+        by MachineStacks.
+
 2011-03-16  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/JavaScriptCore/interpreter/RegisterFile.cpp

@@ -30 +30 @@
 #include "RegisterFile.h"
 
+#include "ConservativeSet.h"
 #include "Interpreter.h"
 #include "JSGlobalData.h"
@@ -50 +51 @@
     addToCommittedByteCount(-(reinterpret_cast<intptr_t>(m_commitEnd) - reinterpret_cast<intptr_t>(base)));
     m_reservation.deallocate();
+}
+
+void RegisterFile::gatherConservativeRoots(ConservativeRoots& conservativeRoots)
+{
+    for (Register* it = start(); it != end(); ++it) {
+        JSValue v = it->jsValue();
+        if (!v.isCell())
+            continue;
+        conservativeRoots.add(v.asCell());
+    }
 }
 

trunk/Source/JavaScriptCore/interpreter/RegisterFile.h

@@ -114 +114 @@
         RegisterFile(JSGlobalData&, size_t capacity = defaultCapacity, size_t maxGlobals = defaultMaxGlobals);
         ~RegisterFile();
+        
+        void gatherConservativeRoots(ConservativeRoots&);
 
         Register* start() const { return m_start; }

trunk/Source/JavaScriptCore/runtime/ConservativeSet.cpp

@@ -27 +27 @@
 #include "ConservativeSet.h"
 
-#include "Heap.h"
-
 namespace JSC {
 
@@ -54 +52 @@
     ASSERT(isPointerAligned(end));
 
-    for (char** it = static_cast<char**>(begin); it != static_cast<char**>(end); ++it) {
-        if (!m_heap->contains(*it))
-            continue;
-
-        if (m_size == m_capacity)
-            grow();
-
-        m_roots[m_size++] = reinterpret_cast<JSCell*>(*it);
-    }
+    for (char** it = static_cast<char**>(begin); it != static_cast<char**>(end); ++it)
+        add(*it);
 }
 

trunk/Source/JavaScriptCore/runtime/ConservativeSet.h

@@ -27 +27 @@
 #define ConservativeRoots_h
 
+#include "Heap.h"
 #include <wtf/OSAllocator.h>
 #include <wtf/Vector.h>
@@ -42 +43 @@
     ~ConservativeRoots();
 
+    void add(void*);
     void add(void* begin, void* end);
     
@@ -74 +76 @@
 }
 
+inline void ConservativeRoots::add(void* p)
+{
+    if (!m_heap->contains(p))
+        return;
+
+    if (m_size == m_capacity)
+        grow();
+
+    m_roots[m_size++] = reinterpret_cast<JSCell*>(p);
+}
+
 inline size_t ConservativeRoots::size()
 {

trunk/Source/JavaScriptCore/runtime/Heap.cpp

@@ -200 +200 @@
 #endif
 
+    void* dummy;
+
     ASSERT(m_operationInProgress == NoOperation);
     if (m_operationInProgress != NoOperation)
@@ -206 +208 @@
     m_operationInProgress = Collection;
 
-    // We gather the conservative set before clearing mark bits, because
+    MarkStack& markStack = m_markStack;
+    HeapRootMarker heapRootMarker(markStack);
+    
+    // We gather conservative roots before clearing mark bits because
     // conservative gathering uses the mark bits from our last mark pass to
     // determine whether a reference is valid.
-    ConservativeRoots conservativeRoots(this);
-    m_machineThreads.gatherConservativeRoots(conservativeRoots);
-    conservativeRoots.add(registerFile().start(), registerFile().end());
+    ConservativeRoots machineThreadRoots(this);
+    m_machineThreads.gatherConservativeRoots(machineThreadRoots, &dummy);
+
+    ConservativeRoots registerFileRoots(this);
+    registerFile().gatherConservativeRoots(registerFileRoots);
 
     m_markedSpace.clearMarks();
 
-    MarkStack& markStack = m_markStack;
-    HeapRootMarker heapRootMarker(markStack);
-
-    markStack.append(conservativeRoots);
+    markStack.append(machineThreadRoots);
+    markStack.drain();
+
+    markStack.append(registerFileRoots);
     markStack.drain();
 

trunk/Source/JavaScriptCore/runtime/MachineStackMarker.cpp

@@ -29 +29 @@
 #include <setjmp.h>
 #include <stdlib.h>
+#include <wtf/StdLibExtras.h>
 
 #if USE(PTHREAD_BASED_QT) && !defined(WTF_USE_PTHREADS)
@@ -84 +85 @@
 
 #endif
+
+using namespace WTF;
 
 namespace JSC {
@@ -245 +248 @@
 #endif
 
-void NEVER_INLINE MachineThreads::gatherFromCurrentThreadInternal(ConservativeRoots& conservativeRoots)
-{
-    void* begin = m_heap->globalData()->stack().current();
-    void* end = m_heap->globalData()->stack().origin();
-    swapIfBackwards(begin, end);
-    conservativeRoots.add(begin, end);
-}
-
 #if COMPILER(GCC)
 #define REGISTER_BUFFER_ALIGNMENT __attribute__ ((aligned (sizeof(void*))))
@@ -259 +254 @@
 #endif
 
-void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots)
+void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoots, void* stackCurrent)
 {
     // setjmp forces volatile registers onto the stack
@@ -272 +267 @@
 #endif
 
-    gatherFromCurrentThreadInternal(conservativeRoots);
+    void* registersBegin = &registers;
+    void* registersEnd = reinterpret_cast<void*>(roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(&registers + 1)));
+    swapIfBackwards(registersBegin, registersEnd);
+    conservativeRoots.add(registersBegin, registersEnd);
+
+    void* stackBegin = stackCurrent;
+    void* stackEnd = m_heap->globalData()->stack().origin();
+    swapIfBackwards(stackBegin, stackEnd);
+    conservativeRoots.add(stackBegin, stackEnd);
 }
 
@@ -457 +460 @@
 #endif
 
-void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots)
-{
-    gatherFromCurrentThread(conservativeRoots);
+void MachineThreads::gatherConservativeRoots(ConservativeRoots& conservativeRoots, void* stackCurrent)
+{
+    gatherFromCurrentThread(conservativeRoots, stackCurrent);
 
 #if ENABLE(JSC_MULTIPLE_THREADS)

trunk/Source/JavaScriptCore/runtime/MachineStackMarker.h

@@ -41 +41 @@
         ~MachineThreads();
 
-        void gatherConservativeRoots(ConservativeRoots&);
+        void gatherConservativeRoots(ConservativeRoots&, void* stackCurrent);
 
 #if ENABLE(JSC_MULTIPLE_THREADS)
@@ -49 +49 @@
 
     private:
-        void gatherFromCurrentThread(ConservativeRoots&);
-        void gatherFromCurrentThreadInternal(ConservativeRoots&);
+        void gatherFromCurrentThread(ConservativeRoots&, void* stackCurrent);
 
 #if ENABLE(JSC_MULTIPLE_THREADS)

trunk/Source/JavaScriptCore/runtime/MarkStack.cpp

@@ -27 +27 @@
 #include "MarkStack.h"
 
+#include "ConservativeSet.h"
 #include "Heap.h"
 #include "JSArray.h"
 #include "JSCell.h"
+#include "JSObject.h"
+#include "ScopeChain.h"
 #include "Structure.h"
 
@@ -41 +44 @@
     m_values.shrinkAllocation(s_pageSize);
     m_markSets.shrinkAllocation(s_pageSize);
+}
+
+void MarkStack::append(ConservativeRoots& conservativeRoots)
+{
+    JSCell** roots = conservativeRoots.roots();
+    size_t size = conservativeRoots.size();
+    for (size_t i = 0; i < size; ++i)
+        internalAppend(roots[i]);
 }
 

trunk/Source/JavaScriptCore/runtime/MarkStack.h

@@ -27 +27 @@
 #define MarkStack_h
 
-#include "ConservativeSet.h"
 #include "JSValue.h"
 #include "Register.h"
@@ -37 +36 @@
 namespace JSC {
 
+    class ConservativeRoots;
     class JSGlobalData;
     class Register;
@@ -80 +80 @@
         }
         
-        void append(ConservativeRoots& conservativeRoots)
-        {
-            JSCell** roots = conservativeRoots.roots();
-            size_t size = conservativeRoots.size();
-            for (size_t i = 0; i < size; ++i)
-                internalAppend(roots[i]);
-        }
+        void append(ConservativeRoots&);
 
         void drain();

trunk/Source/JavaScriptCore/runtime/MarkedBlock.h

@@ -25 +25 @@
 #include <wtf/Bitmap.h>
 #include <wtf/PageAllocationAligned.h>
+#include <wtf/StdLibExtras.h>
 
 namespace JSC {
@@ -35 +36 @@
 
     static const size_t KB = 1024;
-
-    // Efficient implementation that takes advantage of powers of two.
-    template<size_t divisor> inline size_t roundUpToMultipleOf(size_t x)
-    {
-        COMPILE_ASSERT(divisor && !(divisor & (divisor - 1)), divisor_is_a_power_of_two);
-
-        size_t remainderMask = divisor - 1;
-        return (x + remainderMask) & ~remainderMask;
-    }
 
     class MarkedBlock {
@@ -110 +102 @@
     inline size_t MarkedBlock::firstAtom()
     {
-        return roundUpToMultipleOf<atomSize>(sizeof(MarkedBlock)) / atomSize;
+        return WTF::roundUpToMultipleOf<atomSize>(sizeof(MarkedBlock)) / atomSize;
     }
 

trunk/Source/JavaScriptCore/wtf/StdLibExtras.h

@@ -115 +115 @@
 #define WTF_ARRAY_LENGTH(array) sizeof(::WTF::ArrayLengthHelperFunction(array))
 
+// Efficient implementation that takes advantage of powers of two.
+template<size_t divisor> inline size_t roundUpToMultipleOf(size_t x)
+{
+    COMPILE_ASSERT(divisor && !(divisor & (divisor - 1)), divisor_is_a_power_of_two);
+
+    size_t remainderMask = divisor - 1;
+    return (x + remainderMask) & ~remainderMask;
+}
+
 } // namespace WTF