|
| 1 | +2020-10-28 Saam Barati < [email protected]> |
| 2 | + |
| 3 | + Better cache our serialization of the outer TDZ environment when creating FunctionExecutables during bytecode generation |
| 4 | + https://bugs.webkit.org/show_bug.cgi?id=199866 |
| 5 | + <rdar://problem/53333108> |
| 6 | + |
| 7 | + Reviewed by Tadeu Zagallo. |
| 8 | + |
| 9 | + This patch removes performance pathologies regarding programs with |
| 10 | + many variables under TDZ (let/const). We had an algorithm for caching |
| 11 | + the results of gathering all variables under TDZ, but that algorithm |
| 12 | + wasn't nearly aggressive enough in its caching. This lead us to worst |
| 13 | + case quadratic runtime, which could happens in practice for large functions. |
| 14 | + |
| 15 | + There are a few fixes here: |
| 16 | + - Instead of flattening the entire TDZ stack, and caching that result, |
| 17 | + we now cache each stack entry individually. So as you push/pop to the |
| 18 | + TDZ environment stack, we no longer invalidate everything. Instead, we |
| 19 | + will just need to cache the newly pushed entry. We also no longer invalidate |
| 20 | + the cache for lifting a TDZ check. The compromise here is we may emit |
| 21 | + more runtime TDZ checks for closure variables. This is better than N^2 |
| 22 | + bytecode compile time perf, since a well predicted branch for a TDZ |
| 23 | + check is essentially free. |
| 24 | + - We no longer transform the CompactTDZEnvironment (formerly CompactVariableEnvironment) |
| 25 | + from a Vector into a HashSet each time we generate code for an inner function. Instead, |
| 26 | + CompactTDZEnvironment can be in two modes: compact and inflated. It starts life off in |
| 27 | + compact mode (a vector), and will turn into an inflated mode if it's ever needed. Once |
| 28 | + inflated, it'll stay this way until it's destructed. This improves our algorithm from being |
| 29 | + O(EnvSize * NumFunctions) to O(EnvSize) at the cost of using more space in a HashTable versus a |
| 30 | + Vector. In the future, we could consider just binary searching through this Vector, and never using |
| 31 | + a hash table. |
| 32 | + |
| 33 | + * bytecode/UnlinkedFunctionExecutable.cpp: |
| 34 | + (JSC::generateUnlinkedFunctionCodeBlock): |
| 35 | + (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable): |
| 36 | + * bytecode/UnlinkedFunctionExecutable.h: |
| 37 | + * bytecompiler/BytecodeGenerator.cpp: |
| 38 | + (JSC::BytecodeGenerator::BytecodeGenerator): |
| 39 | + (JSC::BytecodeGenerator::popLexicalScopeInternal): |
| 40 | + (JSC::BytecodeGenerator::needsTDZCheck): |
| 41 | + (JSC::BytecodeGenerator::liftTDZCheckIfPossible): |
| 42 | + (JSC::BytecodeGenerator::pushTDZVariables): |
| 43 | + (JSC::BytecodeGenerator::getVariablesUnderTDZ): |
| 44 | + (JSC::BytecodeGenerator::preserveTDZStack): |
| 45 | + (JSC::BytecodeGenerator::restoreTDZStack): |
| 46 | + (JSC::BytecodeGenerator::emitNewInstanceFieldInitializerFunction): |
| 47 | + * bytecompiler/BytecodeGenerator.h: |
| 48 | + (JSC::BytecodeGenerator::generate): |
| 49 | + (JSC::BytecodeGenerator::makeFunction): |
| 50 | + * debugger/DebuggerCallFrame.cpp: |
| 51 | + (JSC::DebuggerCallFrame::evaluateWithScopeExtension): |
| 52 | + * interpreter/Interpreter.cpp: |
| 53 | + (JSC::eval): |
| 54 | + * parser/Parser.h: |
| 55 | + (JSC::Parser<LexerType>::parse): |
| 56 | + (JSC::parse): |
| 57 | + * parser/VariableEnvironment.cpp: |
| 58 | + (JSC::CompactTDZEnvironment::sortCompact): |
| 59 | + (JSC::CompactTDZEnvironment::CompactTDZEnvironment): |
| 60 | + (JSC::CompactTDZEnvironment::operator== const): |
| 61 | + (JSC::CompactTDZEnvironment::toTDZEnvironmentSlow const): |
| 62 | + (JSC::CompactTDZEnvironmentMap::get): |
| 63 | + (JSC::CompactTDZEnvironmentMap::Handle::~Handle): |
| 64 | + (JSC::CompactTDZEnvironmentMap::Handle::Handle): |
| 65 | + (JSC::CompactVariableEnvironment::CompactVariableEnvironment): Deleted. |
| 66 | + (JSC::CompactVariableEnvironment::operator== const): Deleted. |
| 67 | + (JSC::CompactVariableEnvironment::toVariableEnvironment const): Deleted. |
| 68 | + (JSC::CompactVariableMap::get): Deleted. |
| 69 | + (JSC::CompactVariableMap::Handle::~Handle): Deleted. |
| 70 | + (JSC::CompactVariableMap::Handle::Handle): Deleted. |
| 71 | + * parser/VariableEnvironment.h: |
| 72 | + (JSC::CompactTDZEnvironment::toTDZEnvironment const): |
| 73 | + (JSC::CompactTDZEnvironmentKey::CompactTDZEnvironmentKey): |
| 74 | + (JSC::CompactTDZEnvironmentKey::hash): |
| 75 | + (JSC::CompactTDZEnvironmentKey::equal): |
| 76 | + (JSC::CompactTDZEnvironmentKey::makeDeletedValue): |
| 77 | + (JSC::CompactTDZEnvironmentKey::isHashTableDeletedValue const): |
| 78 | + (JSC::CompactTDZEnvironmentKey::environment): |
| 79 | + (WTF::HashTraits<JSC::CompactTDZEnvironmentKey>::emptyValue): |
| 80 | + (WTF::HashTraits<JSC::CompactTDZEnvironmentKey>::isEmptyValue): |
| 81 | + (WTF::HashTraits<JSC::CompactTDZEnvironmentKey>::constructDeletedValue): |
| 82 | + (WTF::HashTraits<JSC::CompactTDZEnvironmentKey>::isDeletedValue): |
| 83 | + (JSC::CompactTDZEnvironmentMap::Handle::environment const): |
| 84 | + (JSC::CompactVariableEnvironment::hash const): Deleted. |
| 85 | + (JSC::CompactVariableMapKey::CompactVariableMapKey): Deleted. |
| 86 | + (JSC::CompactVariableMapKey::hash): Deleted. |
| 87 | + (JSC::CompactVariableMapKey::equal): Deleted. |
| 88 | + (JSC::CompactVariableMapKey::makeDeletedValue): Deleted. |
| 89 | + (JSC::CompactVariableMapKey::isHashTableDeletedValue const): Deleted. |
| 90 | + (JSC::CompactVariableMapKey::isHashTableEmptyValue const): Deleted. |
| 91 | + (JSC::CompactVariableMapKey::environment): Deleted. |
| 92 | + (WTF::HashTraits<JSC::CompactVariableMapKey>::emptyValue): Deleted. |
| 93 | + (WTF::HashTraits<JSC::CompactVariableMapKey>::isEmptyValue): Deleted. |
| 94 | + (WTF::HashTraits<JSC::CompactVariableMapKey>::constructDeletedValue): Deleted. |
| 95 | + (WTF::HashTraits<JSC::CompactVariableMapKey>::isDeletedValue): Deleted. |
| 96 | + (JSC::CompactVariableMap::Handle::Handle): Deleted. |
| 97 | + (JSC::CompactVariableMap::Handle::operator=): Deleted. |
| 98 | + (JSC::CompactVariableMap::Handle::operator bool const): Deleted. |
| 99 | + (JSC::CompactVariableMap::Handle::environment const): Deleted. |
| 100 | + (JSC::CompactVariableMap::Handle::swap): Deleted. |
| 101 | + * runtime/CachedTypes.cpp: |
| 102 | + (JSC::Decoder::handleForTDZEnvironment const): |
| 103 | + (JSC::Decoder::setHandleForTDZEnvironment): |
| 104 | + (JSC::CachedCompactTDZEnvironment::encode): |
| 105 | + (JSC::CachedCompactTDZEnvironment::decode const): |
| 106 | + (JSC::CachedCompactTDZEnvironmentMapHandle::encode): |
| 107 | + (JSC::CachedCompactTDZEnvironmentMapHandle::decode const): |
| 108 | + (JSC::CachedFunctionExecutableRareData::decode const): |
| 109 | + (JSC::Decoder::handleForEnvironment const): Deleted. |
| 110 | + (JSC::Decoder::setHandleForEnvironment): Deleted. |
| 111 | + (JSC::CachedCompactVariableEnvironment::encode): Deleted. |
| 112 | + (JSC::CachedCompactVariableEnvironment::decode const): Deleted. |
| 113 | + (JSC::CachedCompactVariableMapHandle::encode): Deleted. |
| 114 | + (JSC::CachedCompactVariableMapHandle::decode const): Deleted. |
| 115 | + * runtime/CachedTypes.h: |
| 116 | + * runtime/CodeCache.cpp: |
| 117 | + (JSC::generateUnlinkedCodeBlockImpl): |
| 118 | + (JSC::generateUnlinkedCodeBlock): |
| 119 | + (JSC::generateUnlinkedCodeBlockForDirectEval): |
| 120 | + (JSC::recursivelyGenerateUnlinkedCodeBlockForProgram): |
| 121 | + (JSC::recursivelyGenerateUnlinkedCodeBlockForModuleProgram): |
| 122 | + (JSC::CodeCache::getUnlinkedGlobalCodeBlock): |
| 123 | + * runtime/CodeCache.h: |
| 124 | + * runtime/Completion.cpp: |
| 125 | + (JSC::generateProgramBytecode): |
| 126 | + (JSC::generateModuleBytecode): |
| 127 | + * runtime/DirectEvalExecutable.cpp: |
| 128 | + (JSC::DirectEvalExecutable::create): |
| 129 | + * runtime/DirectEvalExecutable.h: |
| 130 | + * runtime/JSScope.cpp: |
| 131 | + (JSC::JSScope::collectClosureVariablesUnderTDZ): |
| 132 | + * runtime/JSScope.h: |
| 133 | + * runtime/VM.cpp: |
| 134 | + (JSC::VM::VM): |
| 135 | + * runtime/VM.h: |
| 136 | + |
1 | 137 | 2020-10-28 Robin Morisset < [email protected]>
|
2 | 138 |
|
3 | 139 | DFGIntegerRangeOptimization is wrong for Upsilon (as 'shadow' nodes are not in SSA form)
|
|
0 commit comments