Diff
Modified: trunk/JSTests/ChangeLog (241570 => 241571)
--- trunk/JSTests/ChangeLog 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/JSTests/ChangeLog 2019-02-15 00:06:30 UTC (rev 241571)
@@ -1,3 +1,13 @@
+2019-02-14 Saam Barati <sbar...@apple.com>
+
+ Cache the results of BytecodeGenerator::getVariablesUnderTDZ
+ https://bugs.webkit.org/show_bug.cgi?id=194583
+ <rdar://problem/48028140>
+
+ Reviewed by Yusuke Suzuki.
+
+ * microbenchmarks/cache-get-variables-under-tdz-in-bytecode-generator.js: Added.
+
2019-02-08 Yusuke Suzuki <ysuz...@apple.com>
[JSC] String.fromCharCode's slow path always generates 16bit string
Added: trunk/JSTests/microbenchmarks/cache-get-variables-under-tdz-in-bytecode-generator.js (0 => 241571)
--- trunk/JSTests/microbenchmarks/cache-get-variables-under-tdz-in-bytecode-generator.js (rev 0)
+++ trunk/JSTests/microbenchmarks/cache-get-variables-under-tdz-in-bytecode-generator.js 2019-02-15 00:06:30 UTC (rev 241571)
@@ -0,0 +1,20 @@
+//@ runDefault
+
+let script = "(() => {";
+for (let i = 0; i < 1000; ++i) {
+ script += `let x${i} = ${i};\n`;
+}
+
+for (let i = 0; i < 1000; ++i) {
+ script += `function f${i}() { return "foo"; }\n`;
+}
+
+script += "})();"
+
+let start = Date.now();
+for (let i = 0; i < 10; ++i) {
+ $.evalScript(`cacheBust = ${Math.random()}; ${script}`);
+}
+
+if (false)
+ print(Date.now() - start);
Modified: trunk/Source/_javascript_Core/ChangeLog (241570 => 241571)
--- trunk/Source/_javascript_Core/ChangeLog 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-02-15 00:06:30 UTC (rev 241571)
@@ -1,3 +1,40 @@
+2019-02-14 Saam Barati <sbar...@apple.com>
+
+ Cache the results of BytecodeGenerator::getVariablesUnderTDZ
+ https://bugs.webkit.org/show_bug.cgi?id=194583
+ <rdar://problem/48028140>
+
+ Reviewed by Yusuke Suzuki.
+
+ This patch makes it so that getVariablesUnderTDZ caches a result of
+ CompactVariableMap::Handle. getVariablesUnderTDZ is costly when
+ it's called in an environment where there are a lot of variables.
+ This patch makes it so we cache its results. This is profitable when
+ getVariablesUnderTDZ is called repeatedly with the same environment
+ state. This is common since we call this every time we encounter a
+ function definition/_expression_ node.
+
+ * builtins/BuiltinExecutables.cpp:
+ (JSC::BuiltinExecutables::createExecutable):
+ * bytecode/UnlinkedFunctionExecutable.cpp:
+ (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
+ * bytecode/UnlinkedFunctionExecutable.h:
+ * bytecompiler/BytecodeGenerator.cpp:
+ (JSC::BytecodeGenerator::popLexicalScopeInternal):
+ (JSC::BytecodeGenerator::liftTDZCheckIfPossible):
+ (JSC::BytecodeGenerator::pushTDZVariables):
+ (JSC::BytecodeGenerator::getVariablesUnderTDZ):
+ (JSC::BytecodeGenerator::restoreTDZStack):
+ * bytecompiler/BytecodeGenerator.h:
+ (JSC::BytecodeGenerator::makeFunction):
+ * parser/VariableEnvironment.cpp:
+ (JSC::CompactVariableMap::Handle::Handle):
+ (JSC::CompactVariableMap::Handle::operator=):
+ * parser/VariableEnvironment.h:
+ (JSC::CompactVariableMap::Handle::operator bool const):
+ * runtime/CodeCache.cpp:
+ (JSC::CodeCache::getUnlinkedGlobalFunctionExecutable):
+
2019-02-14 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Non-JIT entrypoints should share NativeJITCode per entrypoint type
Modified: trunk/Source/_javascript_Core/builtins/BuiltinExecutables.cpp (241570 => 241571)
--- trunk/Source/_javascript_Core/builtins/BuiltinExecutables.cpp 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/builtins/BuiltinExecutables.cpp 2019-02-15 00:06:30 UTC (rev 241571)
@@ -258,7 +258,7 @@
}
VariableEnvironment dummyTDZVariables;
- UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, &metadata, kind, constructAbility, JSParserScriptMode::Classic, dummyTDZVariables, DerivedContextType::None, isBuiltinDefaultClassConstructor);
+ UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, &metadata, kind, constructAbility, JSParserScriptMode::Classic, vm.m_compactVariableMap->get(dummyTDZVariables), DerivedContextType::None, isBuiltinDefaultClassConstructor);
return functionExecutable;
}
Modified: trunk/Source/_javascript_Core/bytecode/UnlinkedFunctionExecutable.cpp (241570 => 241571)
--- trunk/Source/_javascript_Core/bytecode/UnlinkedFunctionExecutable.cpp 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/bytecode/UnlinkedFunctionExecutable.cpp 2019-02-15 00:06:30 UTC (rev 241571)
@@ -78,7 +78,7 @@
return result;
}
-UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(VM* vm, Structure* structure, const SourceCode& parentSource, FunctionMetadataNode* node, UnlinkedFunctionKind kind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, VariableEnvironment& parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor)
+UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(VM* vm, Structure* structure, const SourceCode& parentSource, FunctionMetadataNode* node, UnlinkedFunctionKind kind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, CompactVariableMap::Handle parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor)
: Base(*vm, structure)
, m_firstLineOffset(node->firstLine() - parentSource.firstLine().oneBasedInt())
, m_lineCount(node->lastLine() - node->firstLine())
@@ -107,7 +107,7 @@
, m_ecmaName(node->ecmaName())
, m_inferredName(node->inferredName())
, m_classSource(node->classSource())
- , m_parentScopeTDZVariables(vm->m_compactVariableMap->get(parentScopeTDZVariables))
+ , m_parentScopeTDZVariables(WTFMove(parentScopeTDZVariables))
{
// Make sure these bitfields are adequately wide.
ASSERT(m_constructAbility == static_cast<unsigned>(constructAbility));
Modified: trunk/Source/_javascript_Core/bytecode/UnlinkedFunctionExecutable.h (241570 => 241571)
--- trunk/Source/_javascript_Core/bytecode/UnlinkedFunctionExecutable.h 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/bytecode/UnlinkedFunctionExecutable.h 2019-02-15 00:06:30 UTC (rev 241571)
@@ -67,7 +67,7 @@
return &vm.unlinkedFunctionExecutableSpace.space;
}
- static UnlinkedFunctionExecutable* create(VM* vm, const SourceCode& source, FunctionMetadataNode* node, UnlinkedFunctionKind unlinkedFunctionKind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, VariableEnvironment& parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor = false)
+ static UnlinkedFunctionExecutable* create(VM* vm, const SourceCode& source, FunctionMetadataNode* node, UnlinkedFunctionKind unlinkedFunctionKind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, CompactVariableMap::Handle parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor = false)
{
UnlinkedFunctionExecutable* instance = new (NotNull, allocateCell<UnlinkedFunctionExecutable>(vm->heap))
UnlinkedFunctionExecutable(vm, vm->unlinkedFunctionExecutableStructure.get(), source, node, unlinkedFunctionKind, constructAbility, scriptMode, parentScopeTDZVariables, derivedContextType, isBuiltinDefaultClassConstructor);
@@ -152,7 +152,7 @@
void setSourceMappingURLDirective(const String& sourceMappingURL) { m_sourceMappingURLDirective = sourceMappingURL; }
private:
- UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, VariableEnvironment&, JSC::DerivedContextType, bool isBuiltinDefaultClassConstructor);
+ UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, CompactVariableMap::Handle, JSC::DerivedContextType, bool isBuiltinDefaultClassConstructor);
UnlinkedFunctionExecutable(Decoder&, VariableEnvironment&, const CachedFunctionExecutable&);
unsigned m_firstLineOffset;
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (241570 => 241571)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp 2019-02-15 00:06:30 UTC (rev 241571)
@@ -2195,6 +2195,7 @@
}
m_TDZStack.removeLast();
+ m_cachedVariablesUnderTDZ = { };
}
void BytecodeGenerator::prepareLexicalScopeForNextForLoopIteration(VariableEnvironmentNode* node, RegisterID* loopSymbolTable)
@@ -2814,8 +2815,10 @@
for (unsigned i = m_TDZStack.size(); i--;) {
auto iter = m_TDZStack[i].find(identifier);
if (iter != m_TDZStack[i].end()) {
- if (iter->value == TDZNecessityLevel::Optimize)
+ if (iter->value == TDZNecessityLevel::Optimize) {
+ m_cachedVariablesUnderTDZ = { };
iter->value = TDZNecessityLevel::NotNeeded;
+ }
break;
}
}
@@ -2840,10 +2843,14 @@
map.add(entry.key, entry.value.isFunction() ? TDZNecessityLevel::NotNeeded : level);
m_TDZStack.append(WTFMove(map));
+ m_cachedVariablesUnderTDZ = { };
}
-void BytecodeGenerator::getVariablesUnderTDZ(VariableEnvironment& result)
+CompactVariableMap::Handle BytecodeGenerator::getVariablesUnderTDZ()
{
+ if (m_cachedVariablesUnderTDZ)
+ return m_cachedVariablesUnderTDZ;
+
// We keep track of variablesThatDontNeedTDZ in this algorithm to prevent
// reporting that "x" is under TDZ if this function is called at "...".
//
@@ -2854,18 +2861,21 @@
// }
// let x;
// }
- //
SmallPtrSet<UniquedStringImpl*, 16> variablesThatDontNeedTDZ;
+ VariableEnvironment environment;
for (unsigned i = m_TDZStack.size(); i--; ) {
auto& map = m_TDZStack[i];
for (auto& entry : map) {
if (entry.value != TDZNecessityLevel::NotNeeded) {
if (!variablesThatDontNeedTDZ.contains(entry.key.get()))
- result.add(entry.key.get());
+ environment.add(entry.key.get());
} else
variablesThatDontNeedTDZ.add(entry.key.get());
}
}
+
+ m_cachedVariablesUnderTDZ = m_vm->m_compactVariableMap->get(environment);
+ return m_cachedVariablesUnderTDZ;
}
void BytecodeGenerator::preserveTDZStack(BytecodeGenerator::PreservedTDZStack& preservedStack)
@@ -2876,6 +2886,7 @@
void BytecodeGenerator::restoreTDZStack(const BytecodeGenerator::PreservedTDZStack& preservedStack)
{
m_TDZStack = preservedStack.m_preservedTDZStack;
+ m_cachedVariablesUnderTDZ = { };
}
RegisterID* BytecodeGenerator::emitNewObject(RegisterID* dst)
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (241570 => 241571)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2019-02-15 00:06:30 UTC (rev 241571)
@@ -1137,8 +1137,7 @@
newDerivedContextType = DerivedContextType::DerivedMethodContext;
}
- VariableEnvironment variablesUnderTDZ;
- getVariablesUnderTDZ(variablesUnderTDZ);
+ CompactVariableMap::Handle variablesUnderTDZ = getVariablesUnderTDZ();
// FIXME: These flags, ParserModes and propagation to XXXCodeBlocks should be reorganized.
// https://bugs.webkit.org/show_bug.cgi?id=151547
@@ -1147,10 +1146,10 @@
if (parseMode == SourceParseMode::MethodMode && metadata->constructorKind() != ConstructorKind::None)
constructAbility = ConstructAbility::CanConstruct;
- return UnlinkedFunctionExecutable::create(m_vm, m_scopeNode->source(), metadata, isBuiltinFunction() ? UnlinkedBuiltinFunction : UnlinkedNormalFunction, constructAbility, scriptMode(), variablesUnderTDZ, newDerivedContextType);
+ return UnlinkedFunctionExecutable::create(m_vm, m_scopeNode->source(), metadata, isBuiltinFunction() ? UnlinkedBuiltinFunction : UnlinkedNormalFunction, constructAbility, scriptMode(), WTFMove(variablesUnderTDZ), newDerivedContextType);
}
- void getVariablesUnderTDZ(VariableEnvironment&);
+ CompactVariableMap::Handle getVariablesUnderTDZ();
RegisterID* emitConstructVarargs(RegisterID* dst, RegisterID* func, RegisterID* thisRegister, RegisterID* arguments, RegisterID* firstFreeRegister, int32_t firstVarArgOffset, const JSTextPosition& divot, const JSTextPosition& divotStart, const JSTextPosition& divotEnd, DebuggableCall);
template<typename CallOp>
@@ -1313,6 +1312,8 @@
bool m_needsToUpdateArrowFunctionContext;
DerivedContextType m_derivedContextType { DerivedContextType::None };
+ CompactVariableMap::Handle m_cachedVariablesUnderTDZ;
+
using CatchEntry = std::tuple<TryData*, VirtualRegister, VirtualRegister>;
Vector<CatchEntry> m_catchesToEmit;
};
Modified: trunk/Source/_javascript_Core/parser/VariableEnvironment.cpp (241570 => 241571)
--- trunk/Source/_javascript_Core/parser/VariableEnvironment.cpp 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/parser/VariableEnvironment.cpp 2019-02-15 00:06:30 UTC (rev 241571)
@@ -183,4 +183,27 @@
}
}
+CompactVariableMap::Handle::Handle(const CompactVariableMap::Handle& other)
+{
+ *this = other;
+}
+
+CompactVariableMap::Handle& CompactVariableMap::Handle::operator=(const Handle& other)
+{
+ m_map = other.m_map;
+ m_environment = other.m_environment;
+
+ if (!m_map) {
+ ASSERT(!m_environment);
+ // This happens if `other` were moved into a different handle.
+ return *this;
+ }
+
+ auto iter = m_map->m_map.find(CompactVariableMapKey { *m_environment });
+ RELEASE_ASSERT(iter != m_map->m_map.end());
+ ++iter->value;
+
+ return *this;
+}
+
} // namespace JSC
Modified: trunk/Source/_javascript_Core/parser/VariableEnvironment.h (241570 => 241571)
--- trunk/Source/_javascript_Core/parser/VariableEnvironment.h 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/parser/VariableEnvironment.h 2019-02-15 00:06:30 UTC (rev 241571)
@@ -204,8 +204,9 @@
class CompactVariableMap : public RefCounted<CompactVariableMap> {
public:
class Handle {
- WTF_MAKE_NONCOPYABLE(Handle); // If we wanted to make this copyable, we'd need to do a hashtable lookup and bump the reference count of the map entry.
public:
+ Handle() = default;
+
Handle(CompactVariableEnvironment& environment, CompactVariableMap& map)
: m_environment(&environment)
, m_map(&map)
@@ -218,8 +219,14 @@
ASSERT(!other.m_map);
other.m_environment = nullptr;
}
+
+ Handle(const Handle&);
+ Handle& operator=(const Handle&);
+
~Handle();
+ explicit operator bool() const { return !!m_map; }
+
const CompactVariableEnvironment& environment() const
{
return *m_environment;
@@ -226,7 +233,7 @@
}
private:
- CompactVariableEnvironment* m_environment;
+ CompactVariableEnvironment* m_environment { nullptr };
RefPtr<CompactVariableMap> m_map;
};
Modified: trunk/Source/_javascript_Core/runtime/CodeCache.cpp (241570 => 241571)
--- trunk/Source/_javascript_Core/runtime/CodeCache.cpp 2019-02-15 00:01:53 UTC (rev 241570)
+++ trunk/Source/_javascript_Core/runtime/CodeCache.cpp 2019-02-15 00:06:30 UTC (rev 241571)
@@ -147,7 +147,7 @@
// in the global lexical environment, which we always TDZ check accesses from.
VariableEnvironment emptyTDZVariables;
ConstructAbility constructAbility = constructAbilityForParseMode(metadata->parseMode());
- UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, metadata, UnlinkedNormalFunction, constructAbility, JSParserScriptMode::Classic, emptyTDZVariables, DerivedContextType::None);
+ UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, metadata, UnlinkedNormalFunction, constructAbility, JSParserScriptMode::Classic, vm.m_compactVariableMap->get(emptyTDZVariables), DerivedContextType::None);
functionExecutable->setSourceURLDirective(source.provider()->sourceURL());
functionExecutable->setSourceMappingURLDirective(source.provider()->sourceMappingURL());
