Lets not iterate over the constant pool twice every time we link a code block

https://bugs.webkit.org/show_bug.cgi?id=156517

Reviewed by Mark Lam.

I introduced a second iteration over the constant pool when I implemented
block scoping. I did this because we must clone all the symbol tables when
we link a CodeBlock. We can just do this cloning when setting the constant
registers for the first time. There is no need to iterate over the constant
pool a second time.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finishCreation):
(JSC::CodeBlock::~CodeBlock):
(JSC::CodeBlock::setConstantRegisters):
(JSC::CodeBlock::setAlternative):
* bytecode/CodeBlock.h:
(JSC::CodeBlock::replaceConstant):
(JSC::CodeBlock::setConstantRegisters): Deleted.

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

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 5b9ff37..43af2ea 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,25 @@
+2016-04-12  Saam barati  <sbarati@apple.com>
+
+        Lets not iterate over the constant pool twice every time we link a code block
+        https://bugs.webkit.org/show_bug.cgi?id=156517
+
+        Reviewed by Mark Lam.
+
+        I introduced a second iteration over the constant pool when I implemented
+        block scoping. I did this because we must clone all the symbol tables when
+        we link a CodeBlock. We can just do this cloning when setting the constant
+        registers for the first time. There is no need to iterate over the constant
+        pool a second time.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::finishCreation):
+        (JSC::CodeBlock::~CodeBlock):
+        (JSC::CodeBlock::setConstantRegisters):
+        (JSC::CodeBlock::setAlternative):
+        * bytecode/CodeBlock.h:
+        (JSC::CodeBlock::replaceConstant):
+        (JSC::CodeBlock::setConstantRegisters): Deleted.
+
 2016-04-12  Mark Lam  <mark.lam@apple.com>
 
         ES6: Implement String.prototype.split and RegExp.prototype[@@split].

diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
index 9ef3a46..3efb0f5 100644 (file)
--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
@@ -1886,31 +1886,6 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
             m_constantRegisters[registerIndex].set(*m_vm, this, m_globalObject->jsCellForLinkTimeConstant(type));
     }
 
-#if !ASSERT_DISABLED
-    HashSet<int, WTF::IntHash<int>, WTF::UnsignedWithZeroKeyHashTraits<int>> clonedConstantSymbolTables;
-#endif
-    {
-#if !ASSERT_DISABLED
-        HashSet<SymbolTable*> clonedSymbolTables;
-#endif
-        bool hasTypeProfiler = !!vm.typeProfiler();
-        for (unsigned i = 0; i < m_constantRegisters.size(); i++) {
-            if (m_constantRegisters[i].get().isEmpty())
-                continue;
-            if (SymbolTable* symbolTable = jsDynamicCast<SymbolTable*>(m_constantRegisters[i].get())) {
-                ASSERT(clonedSymbolTables.add(symbolTable).isNewEntry);
-                if (hasTypeProfiler) {
-                    ConcurrentJITLocker locker(symbolTable->m_lock);
-                    symbolTable->prepareForTypeProfiling(locker);
-                }
-                m_constantRegisters[i].set(*m_vm, this, symbolTable->cloneScopePart(*m_vm));
-#if !ASSERT_DISABLED
-                clonedConstantSymbolTables.add(i + FirstConstantRegisterIndex);
-#endif
-            }
-        }
-    }
-
     // We already have the cloned symbol table for the module environment since we need to instantiate
     // the module environments before linking the code block. We replace the stored symbol table with the already cloned one.
     if (UnlinkedModuleProgramCodeBlock* unlinkedModuleProgramCodeBlock = jsDynamicCast<UnlinkedModuleProgramCodeBlock*>(unlinkedCodeBlock)) {
@@ -2106,14 +2081,6 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
         case op_get_array_length:
             CRASH();
 
-#if !ASSERT_DISABLED
-        case op_create_lexical_environment: {
-            int symbolTableIndex = pc[3].u.operand;
-            ASSERT(clonedConstantSymbolTables.contains(symbolTableIndex));
-            break;
-        }
-#endif
-
         case op_resolve_scope: {
             const Identifier& ident = identifier(pc[3].u.operand);
             ResolveType type = static_cast<ResolveType>(pc[4].u.operand);
@@ -2177,7 +2144,6 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
                 // Only do watching if the property we're putting to is not anonymous.
                 if (static_cast<unsigned>(pc[2].u.operand) != UINT_MAX) {
                     int symbolTableIndex = pc[5].u.operand;
-                    ASSERT(clonedConstantSymbolTables.contains(symbolTableIndex));
                     SymbolTable* symbolTable = jsCast<SymbolTable*>(getConstant(symbolTableIndex));
                     const Identifier& ident = identifier(pc[2].u.operand);
                     ConcurrentJITLocker locker(symbolTable->m_lock);
@@ -2248,7 +2214,6 @@ void CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
             }
             case ProfileTypeBytecodeLocallyResolved: {
                 int symbolTableIndex = pc[2].u.operand;
-                ASSERT(clonedConstantSymbolTables.contains(symbolTableIndex));
                 SymbolTable* symbolTable = jsCast<SymbolTable*>(getConstant(symbolTableIndex));
                 const Identifier& ident = identifier(pc[4].u.operand);
                 ConcurrentJITLocker locker(symbolTable->m_lock);
@@ -2419,6 +2384,31 @@ CodeBlock::~CodeBlock()
 #endif // ENABLE(JIT)
 }
 
+void CodeBlock::setConstantRegisters(const Vector<WriteBarrier<Unknown>>& constants, const Vector<SourceCodeRepresentation>& constantsSourceCodeRepresentation)
+{
+    ASSERT(constants.size() == constantsSourceCodeRepresentation.size());
+    size_t count = constants.size();
+    m_constantRegisters.resizeToFit(count);
+    bool hasTypeProfiler = !!m_vm->typeProfiler();
+    for (size_t i = 0; i < count; i++) {
+        JSValue constant = constants[i].get();
+
+        if (!constant.isEmpty()) {
+            if (SymbolTable* symbolTable = jsDynamicCast<SymbolTable*>(constant)) {
+                if (hasTypeProfiler) {
+                    ConcurrentJITLocker locker(symbolTable->m_lock);
+                    symbolTable->prepareForTypeProfiling(locker);
+                }
+                constant = symbolTable->cloneScopePart(*m_vm);
+            }
+        }
+
+        m_constantRegisters[i].set(*m_vm, this, constant);
+    }
+
+    m_constantsSourceCodeRepresentation = constantsSourceCodeRepresentation;
+}
+
 void CodeBlock::setAlternative(VM& vm, CodeBlock* alternative)
 {
     m_alternative.set(vm, this, alternative);

diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h
index 171fccd..c385660 100644 (file)
--- a/Source/JavaScriptCore/bytecode/CodeBlock.h
+++ b/Source/JavaScriptCore/bytecode/CodeBlock.h
@@ -936,15 +936,7 @@ private:
 
     void updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles);
 
-    void setConstantRegisters(const Vector<WriteBarrier<Unknown>>& constants, const Vector<SourceCodeRepresentation>& constantsSourceCodeRepresentation)
-    {
-        ASSERT(constants.size() == constantsSourceCodeRepresentation.size());
-        size_t count = constants.size();
-        m_constantRegisters.resizeToFit(count);
-        for (size_t i = 0; i < count; i++)
-            m_constantRegisters[i].set(*m_vm, this, constants[i].get());
-        m_constantsSourceCodeRepresentation = constantsSourceCodeRepresentation;
-    }
+    void setConstantRegisters(const Vector<WriteBarrier<Unknown>>& constants, const Vector<SourceCodeRepresentation>& constantsSourceCodeRepresentation);
 
     void replaceConstant(int index, JSValue value)
     {