Make SegmentedVector Noncopyable
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Mar 2013 21:02:39 +0000 (21:02 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Mar 2013 21:02:39 +0000 (21:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=112059

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Copying a SegmentedVector is very expensive, and really shouldn't
be necessary.  So I've taken the one place where we currently copy
and replaced it with a regular Vector, and replaced the address
dependent logic with a indexing ref instead.

* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::newLabelScope):
(JSC::BytecodeGenerator::emitComplexJumpScopes):
* bytecompiler/BytecodeGenerator.h:
(BytecodeGenerator):
* bytecompiler/LabelScope.h:
(JSC):
(JSC::LabelScopePtr::LabelScopePtr):
(LabelScopePtr):
(JSC::LabelScopePtr::operator=):
(JSC::LabelScopePtr::~LabelScopePtr):
(JSC::LabelScopePtr::operator*):
(JSC::LabelScopePtr::operator->):
* bytecompiler/NodesCodegen.cpp:
(JSC::DoWhileNode::emitBytecode):
(JSC::WhileNode::emitBytecode):
(JSC::ForNode::emitBytecode):
(JSC::ForInNode::emitBytecode):
(JSC::SwitchNode::emitBytecode):
(JSC::LabelNode::emitBytecode):

Source/WTF:

Copying a SegmentedVector can be extraordinarily expensive, so we beat
it with the Noncopyable stick - that way we can ensure that if anyone
wants an actual copy they know what they're doing.

* wtf/SegmentedVector.h:
(SegmentedVector):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@145401 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
Source/JavaScriptCore/bytecompiler/LabelScope.h
Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/SegmentedVector.h

index 9116a4872d55772bebcb5a87606e438d9ff2a926..b661405e5ddfdc9674be53361246535a54eedbf2 100644 (file)
@@ -1,3 +1,36 @@
+2013-03-11  Oliver Hunt  <oliver@apple.com>
+
+        Make SegmentedVector Noncopyable
+        https://bugs.webkit.org/show_bug.cgi?id=112059
+
+        Reviewed by Geoffrey Garen.
+
+        Copying a SegmentedVector is very expensive, and really shouldn't
+        be necessary.  So I've taken the one place where we currently copy
+        and replaced it with a regular Vector, and replaced the address
+        dependent logic with a indexing ref instead.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::newLabelScope):
+        (JSC::BytecodeGenerator::emitComplexJumpScopes):
+        * bytecompiler/BytecodeGenerator.h:
+        (BytecodeGenerator):
+        * bytecompiler/LabelScope.h:
+        (JSC):
+        (JSC::LabelScopePtr::LabelScopePtr):
+        (LabelScopePtr):
+        (JSC::LabelScopePtr::operator=):
+        (JSC::LabelScopePtr::~LabelScopePtr):
+        (JSC::LabelScopePtr::operator*):
+        (JSC::LabelScopePtr::operator->):
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::DoWhileNode::emitBytecode):
+        (JSC::WhileNode::emitBytecode):
+        (JSC::ForNode::emitBytecode):
+        (JSC::ForInNode::emitBytecode):
+        (JSC::SwitchNode::emitBytecode):
+        (JSC::LabelNode::emitBytecode):
+
 2013-03-10  Andreas Kling  <akling@apple.com>
 
         SpeculativeJIT should use OwnPtr<SlowPathGenerator>.
index d0cd27932679bd0531350f2cd04d4a32f257118a..4636e7c5965c61d1683d4cac3af2c76942c65628 100644 (file)
@@ -639,7 +639,7 @@ RegisterID* BytecodeGenerator::newTemporary()
     return result;
 }
 
-PassRefPtr<LabelScope> BytecodeGenerator::newLabelScope(LabelScope::Type type, const Identifier* name)
+LabelScopePtr BytecodeGenerator::newLabelScope(LabelScope::Type type, const Identifier* name)
 {
     // Reclaim free label scopes.
     while (m_labelScopes.size() && !m_labelScopes.last().refCount())
@@ -648,7 +648,7 @@ PassRefPtr<LabelScope> BytecodeGenerator::newLabelScope(LabelScope::Type type, c
     // Allocate new label scope.
     LabelScope scope(type, name, scopeDepth(), newLabel(), type == LabelScope::Loop ? newLabel() : PassRefPtr<Label>()); // Only loops have continue targets.
     m_labelScopes.append(scope);
-    return &m_labelScopes.last();
+    return LabelScopePtr(&m_labelScopes, m_labelScopes.size() - 1);
 }
 
 PassRefPtr<Label> BytecodeGenerator::newLabel()
@@ -2281,7 +2281,7 @@ PassRefPtr<Label> BytecodeGenerator::emitComplexJumpScopes(Label* target, Contro
         Vector<SwitchInfo> savedSwitchContextStack;
         Vector<ForInContext> savedForInContextStack;
         Vector<TryContext> poppedTryContexts;
-        SegmentedVector<LabelScope, 8> savedLabelScopes;
+        LabelScopeStore savedLabelScopes;
         while (topScope > bottomScope && topScope->isFinallyBlock) {
             RefPtr<Label> beforeFinally = emitLabel(newLabel().get());
             
index a5135d71feb72888e929491cd79ba8eb6268b912..f9d2ab71be24fefa6419fabf5baa2d594fb6ee0b 100644 (file)
@@ -331,7 +331,7 @@ namespace JSC {
             return dst == ignoredResult() ? 0 : (dst && dst != src) ? emitMove(dst, src) : src;
         }
 
-        PassRefPtr<LabelScope> newLabelScope(LabelScope::Type, const Identifier* = 0);
+        LabelScopePtr newLabelScope(LabelScope::Type, const Identifier* = 0);
         PassRefPtr<Label> newLabel();
 
         // The emitNode functions are just syntactic sugar for calling
@@ -701,7 +701,7 @@ namespace JSC {
         SegmentedVector<RegisterID, 32> m_calleeRegisters;
         SegmentedVector<RegisterID, 32> m_parameters;
         SegmentedVector<Label, 32> m_labels;
-        SegmentedVector<LabelScope, 8> m_labelScopes;
+        LabelScopeStore m_labelScopes;
         RefPtr<RegisterID> m_lastVar;
         int m_finallyDepth;
         int m_dynamicScopeDepth;
index cc21fffd114b22a6343db8409814076050f7fb82..2df6f1b9bce871b4471b4c59e5d13a92e5f18558 100644 (file)
@@ -49,13 +49,6 @@ namespace JSC {
             , m_continueTarget(continueTarget)
         {
         }
-
-        void ref() { ++m_refCount; }
-        void deref()
-        {
-            --m_refCount;
-            ASSERT(m_refCount >= 0);
-        }
         int refCount() const { return m_refCount; }
 
         Label* breakTarget() const { return m_breakTarget.get(); }
@@ -66,6 +59,15 @@ namespace JSC {
         int scopeDepth() const { return m_scopeDepth; }
 
     private:
+        friend class LabelScopePtr;
+
+        void ref() { ++m_refCount; }
+        void deref()
+        {
+            --m_refCount;
+            ASSERT(m_refCount >= 0);
+        }
+
         int m_refCount;
         Type m_type;
         const Identifier* m_name;
@@ -74,6 +76,57 @@ namespace JSC {
         RefPtr<Label> m_continueTarget;
     };
 
+    typedef Vector<LabelScope, 8> LabelScopeStore;
+
+    class LabelScopePtr {
+    public:
+        LabelScopePtr()
+            : m_owner(0)
+            , m_index(0)
+        {
+        }
+        LabelScopePtr(LabelScopeStore* owner, size_t index)
+            : m_owner(owner)
+            , m_index(index)
+        {
+            m_owner->at(index).ref();
+        }
+
+        LabelScopePtr(const LabelScopePtr& other)
+            : m_owner(other.m_owner)
+            , m_index(other.m_index)
+        {
+            if (m_owner)
+                m_owner->at(m_index).ref();
+        }
+
+        const LabelScopePtr& operator=(const LabelScopePtr& other)
+        {
+            if (other.m_owner)
+                other.m_owner->at(other.m_index).ref();
+            if (m_owner)
+                m_owner->at(m_index).deref();
+            m_owner = other.m_owner;
+            m_index = other.m_index;
+            return *this;
+        }
+
+        ~LabelScopePtr()
+        {
+            if (m_owner)
+                m_owner->at(m_index).deref();
+        }
+
+        LabelScope& operator*() { ASSERT(m_owner); return m_owner->at(m_index); }
+        LabelScope* operator->() { ASSERT(m_owner); return &m_owner->at(m_index); }
+        const LabelScope& operator*() const { ASSERT(m_owner); return m_owner->at(m_index); }
+        const LabelScope* operator->() const { ASSERT(m_owner); return &m_owner->at(m_index); }
+
+    private:
+        LabelScopeStore* m_owner;
+        size_t m_index;
+    };
+
 } // namespace JSC
 
 #endif // LabelScope_h
index a81873f69dbf15d96e7a4655478ffd8932a1647d..3c234d9a0c294a598a9218acbb78b4a0ad052ace 100644 (file)
@@ -1594,7 +1594,7 @@ RegisterID* IfElseNode::emitBytecode(BytecodeGenerator& generator, RegisterID* d
 
 RegisterID* DoWhileNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 {
-    RefPtr<LabelScope> scope = generator.newLabelScope(LabelScope::Loop);
+    LabelScopePtr scope = generator.newLabelScope(LabelScope::Loop);
 
     RefPtr<Label> topOfLoop = generator.newLabel();
     generator.emitLabel(topOfLoop.get());
@@ -1620,7 +1620,7 @@ RegisterID* DoWhileNode::emitBytecode(BytecodeGenerator& generator, RegisterID*
 
 RegisterID* WhileNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 {
-    RefPtr<LabelScope> scope = generator.newLabelScope(LabelScope::Loop);
+    LabelScopePtr scope = generator.newLabelScope(LabelScope::Loop);
     RefPtr<Label> topOfLoop = generator.newLabel();
 
     generator.emitDebugHook(WillExecuteStatement, m_expr->lineNo(), m_expr->lineNo(), m_expr->columnNo());
@@ -1656,7 +1656,7 @@ RegisterID* WhileNode::emitBytecode(BytecodeGenerator& generator, RegisterID* ds
 
 RegisterID* ForNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 {
-    RefPtr<LabelScope> scope = generator.newLabelScope(LabelScope::Loop);
+    LabelScopePtr scope = generator.newLabelScope(LabelScope::Loop);
 
     generator.emitDebugHook(WillExecuteStatement, firstLine(), lastLine(), column());
 
@@ -1701,7 +1701,7 @@ RegisterID* ForNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 
 RegisterID* ForInNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 {
-    RefPtr<LabelScope> scope = generator.newLabelScope(LabelScope::Loop);
+    LabelScopePtr scope = generator.newLabelScope(LabelScope::Loop);
 
     if (!m_lexpr->isLocation())
         return emitThrowReferenceError(generator, "Left side of for-in statement is not a reference.");
@@ -2010,7 +2010,7 @@ RegisterID* SwitchNode::emitBytecode(BytecodeGenerator& generator, RegisterID* d
 {
     generator.emitDebugHook(WillExecuteStatement, firstLine(), lastLine(), column());
     
-    RefPtr<LabelScope> scope = generator.newLabelScope(LabelScope::Switch);
+    LabelScopePtr scope = generator.newLabelScope(LabelScope::Switch);
 
     RefPtr<RegisterID> r0 = generator.emitNode(m_expr);
     RegisterID* r1 = m_block->emitBytecodeForBlock(generator, r0.get(), dst);
@@ -2027,7 +2027,7 @@ RegisterID* LabelNode::emitBytecode(BytecodeGenerator& generator, RegisterID* ds
 
     ASSERT(!generator.breakTarget(m_name));
 
-    RefPtr<LabelScope> scope = generator.newLabelScope(LabelScope::NamedLabel, &m_name);
+    LabelScopePtr scope = generator.newLabelScope(LabelScope::NamedLabel, &m_name);
     RegisterID* r0 = generator.emitNode(dst, m_statement);
 
     generator.emitLabel(scope->breakTarget());
index e5e97c33c42aed9a024c82e589f728589c01ca6a..6b6a09d02b4d885309fa6dc5e7bd2da4102fa8f4 100644 (file)
@@ -1,3 +1,17 @@
+2013-03-11  Oliver Hunt  <oliver@apple.com>
+
+        Make SegmentedVector Noncopyable
+        https://bugs.webkit.org/show_bug.cgi?id=112059
+
+        Reviewed by Geoffrey Garen.
+
+        Copying a SegmentedVector can be extraordinarily expensive, so we beat
+        it with the Noncopyable stick - that way we can ensure that if anyone
+        wants an actual copy they know what they're doing.
+
+        * wtf/SegmentedVector.h:
+        (SegmentedVector):
+
 2013-03-08  Benjamin Poulain  <benjamin@webkit.org>
 
         [Mac] Add a feature flag for 'view-mode' Media Feature, disable it on Mac
index b5bf9990afaed2007df73daf85bd5744ae3ec67f..9065f2f153593568ab18a704ed20fa8f2cc3cde9 100644 (file)
@@ -29,6 +29,7 @@
 #ifndef SegmentedVector_h
 #define SegmentedVector_h
 
+#include <wtf/Noncopyable.h>
 #include <wtf/Vector.h>
 
 namespace WTF {
@@ -104,6 +105,8 @@ namespace WTF {
     template <typename T, size_t SegmentSize, size_t InlineCapacity>
     class SegmentedVector {
         friend class SegmentedVectorIterator<T, SegmentSize, InlineCapacity>;
+        WTF_MAKE_NONCOPYABLE(SegmentedVector);
+
     public:
         typedef SegmentedVectorIterator<T, SegmentSize, InlineCapacity> Iterator;