From 7ab014c8503d74e1ee239aaba29f77d597a07cef Mon Sep 17 00:00:00 2001 From: "sbarati@apple.com" Date: Tue, 12 Apr 2016 22:42:06 +0000 Subject: [PATCH] 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 --- Source/JavaScriptCore/ChangeLog | 22 ++++++++++ Source/JavaScriptCore/bytecode/CodeBlock.cpp | 60 ++++++++++++---------------- Source/JavaScriptCore/bytecode/CodeBlock.h | 10 +---- 3 files changed, 48 insertions(+), 44 deletions(-) diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index 5b9ff37..43af2ea 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,25 @@ +2016-04-12 Saam barati + + 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 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 --- 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, WTF::UnsignedWithZeroKeyHashTraits> clonedConstantSymbolTables; -#endif - { -#if !ASSERT_DISABLED - HashSet 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(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(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(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(pc[2].u.operand) != UINT_MAX) { int symbolTableIndex = pc[5].u.operand; - ASSERT(clonedConstantSymbolTables.contains(symbolTableIndex)); SymbolTable* symbolTable = jsCast(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(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>& constants, const Vector& 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(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 --- 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>& constants, const Vector& 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>& constants, const Vector& constantsSourceCodeRepresentation); void replaceConstant(int index, JSValue value) { -- 1.8.3.1