Changeset 241571 in webkit


Ignore:
Timestamp:
Feb 14, 2019 4:06:30 PM (5 years ago)
Author:
sbarati@apple.com
Message:

Cache the results of BytecodeGenerator::getVariablesUnderTDZ
https://bugs.webkit.org/show_bug.cgi?id=194583
<rdar://problem/48028140>

Reviewed by Yusuke Suzuki.

JSTests:

  • microbenchmarks/cache-get-variables-under-tdz-in-bytecode-generator.js: Added.

Source/JavaScriptCore:

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):

Location:
trunk
Files:
1 added
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r241233 r241571  
     12019-02-14  Saam Barati  <sbarati@apple.com>
     2
     3        Cache the results of BytecodeGenerator::getVariablesUnderTDZ
     4        https://bugs.webkit.org/show_bug.cgi?id=194583
     5        <rdar://problem/48028140>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        * microbenchmarks/cache-get-variables-under-tdz-in-bytecode-generator.js: Added.
     10
    1112019-02-08  Yusuke Suzuki  <ysuzuki@apple.com>
    212
  • trunk/Source/JavaScriptCore/ChangeLog

    r241560 r241571  
     12019-02-14  Saam Barati  <sbarati@apple.com>
     2
     3        Cache the results of BytecodeGenerator::getVariablesUnderTDZ
     4        https://bugs.webkit.org/show_bug.cgi?id=194583
     5        <rdar://problem/48028140>
     6
     7        Reviewed by Yusuke Suzuki.
     8
     9        This patch makes it so that getVariablesUnderTDZ caches a result of
     10        CompactVariableMap::Handle. getVariablesUnderTDZ is costly when
     11        it's called in an environment where there are a lot of variables.
     12        This patch makes it so we cache its results. This is profitable when
     13        getVariablesUnderTDZ is called repeatedly with the same environment
     14        state. This is common since we call this every time we encounter a
     15        function definition/expression node.
     16
     17        * builtins/BuiltinExecutables.cpp:
     18        (JSC::BuiltinExecutables::createExecutable):
     19        * bytecode/UnlinkedFunctionExecutable.cpp:
     20        (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
     21        * bytecode/UnlinkedFunctionExecutable.h:
     22        * bytecompiler/BytecodeGenerator.cpp:
     23        (JSC::BytecodeGenerator::popLexicalScopeInternal):
     24        (JSC::BytecodeGenerator::liftTDZCheckIfPossible):
     25        (JSC::BytecodeGenerator::pushTDZVariables):
     26        (JSC::BytecodeGenerator::getVariablesUnderTDZ):
     27        (JSC::BytecodeGenerator::restoreTDZStack):
     28        * bytecompiler/BytecodeGenerator.h:
     29        (JSC::BytecodeGenerator::makeFunction):
     30        * parser/VariableEnvironment.cpp:
     31        (JSC::CompactVariableMap::Handle::Handle):
     32        (JSC::CompactVariableMap::Handle::operator=):
     33        * parser/VariableEnvironment.h:
     34        (JSC::CompactVariableMap::Handle::operator bool const):
     35        * runtime/CodeCache.cpp:
     36        (JSC::CodeCache::getUnlinkedGlobalFunctionExecutable):
     37
    1382019-02-14  Yusuke Suzuki  <ysuzuki@apple.com>
    239
  • trunk/Source/JavaScriptCore/builtins/BuiltinExecutables.cpp

    r240228 r241571  
    259259
    260260    VariableEnvironment dummyTDZVariables;
    261     UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, &metadata, kind, constructAbility, JSParserScriptMode::Classic, dummyTDZVariables, DerivedContextType::None, isBuiltinDefaultClassConstructor);
     261    UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, &metadata, kind, constructAbility, JSParserScriptMode::Classic, vm.m_compactVariableMap->get(dummyTDZVariables), DerivedContextType::None, isBuiltinDefaultClassConstructor);
    262262    return functionExecutable;
    263263}
  • trunk/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp

    r241552 r241571  
    7979}
    8080
    81 UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(VM* vm, Structure* structure, const SourceCode& parentSource, FunctionMetadataNode* node, UnlinkedFunctionKind kind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, VariableEnvironment& parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor)
     81UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(VM* vm, Structure* structure, const SourceCode& parentSource, FunctionMetadataNode* node, UnlinkedFunctionKind kind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, CompactVariableMap::Handle parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor)
    8282    : Base(*vm, structure)
    8383    , m_firstLineOffset(node->firstLine() - parentSource.firstLine().oneBasedInt())
     
    108108    , m_inferredName(node->inferredName())
    109109    , m_classSource(node->classSource())
    110     , m_parentScopeTDZVariables(vm->m_compactVariableMap->get(parentScopeTDZVariables))
     110    , m_parentScopeTDZVariables(WTFMove(parentScopeTDZVariables))
    111111{
    112112    // Make sure these bitfields are adequately wide.
  • trunk/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h

    r241552 r241571  
    6868    }
    6969
    70     static UnlinkedFunctionExecutable* create(VM* vm, const SourceCode& source, FunctionMetadataNode* node, UnlinkedFunctionKind unlinkedFunctionKind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, VariableEnvironment& parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor = false)
     70    static UnlinkedFunctionExecutable* create(VM* vm, const SourceCode& source, FunctionMetadataNode* node, UnlinkedFunctionKind unlinkedFunctionKind, ConstructAbility constructAbility, JSParserScriptMode scriptMode, CompactVariableMap::Handle parentScopeTDZVariables, DerivedContextType derivedContextType, bool isBuiltinDefaultClassConstructor = false)
    7171    {
    7272        UnlinkedFunctionExecutable* instance = new (NotNull, allocateCell<UnlinkedFunctionExecutable>(vm->heap))
     
    153153
    154154private:
    155     UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, VariableEnvironment&,  JSC::DerivedContextType, bool isBuiltinDefaultClassConstructor);
     155    UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, CompactVariableMap::Handle,  JSC::DerivedContextType, bool isBuiltinDefaultClassConstructor);
    156156    UnlinkedFunctionExecutable(Decoder&, VariableEnvironment&, const CachedFunctionExecutable&);
    157157
  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r240981 r241571  
    21962196
    21972197    m_TDZStack.removeLast();
     2198    m_cachedVariablesUnderTDZ = { };
    21982199}
    21992200
     
    28152816        auto iter = m_TDZStack[i].find(identifier);
    28162817        if (iter != m_TDZStack[i].end()) {
    2817             if (iter->value == TDZNecessityLevel::Optimize)
     2818            if (iter->value == TDZNecessityLevel::Optimize) {
     2819                m_cachedVariablesUnderTDZ = { };
    28182820                iter->value = TDZNecessityLevel::NotNeeded;
     2821            }
    28192822            break;
    28202823        }
     
    28412844
    28422845    m_TDZStack.append(WTFMove(map));
    2843 }
    2844 
    2845 void BytecodeGenerator::getVariablesUnderTDZ(VariableEnvironment& result)
    2846 {
     2846    m_cachedVariablesUnderTDZ = { };
     2847}
     2848
     2849CompactVariableMap::Handle BytecodeGenerator::getVariablesUnderTDZ()
     2850{
     2851    if (m_cachedVariablesUnderTDZ)
     2852        return m_cachedVariablesUnderTDZ;
     2853
    28472854    // We keep track of variablesThatDontNeedTDZ in this algorithm to prevent
    28482855    // reporting that "x" is under TDZ if this function is called at "...".
     
    28552862    //         let x;
    28562863    //     }
    2857     //
    28582864    SmallPtrSet<UniquedStringImpl*, 16> variablesThatDontNeedTDZ;
     2865    VariableEnvironment environment;
    28592866    for (unsigned i = m_TDZStack.size(); i--; ) {
    28602867        auto& map = m_TDZStack[i];
     
    28622869            if (entry.value != TDZNecessityLevel::NotNeeded) {
    28632870                if (!variablesThatDontNeedTDZ.contains(entry.key.get()))
    2864                     result.add(entry.key.get());
     2871                    environment.add(entry.key.get());
    28652872            } else
    28662873                variablesThatDontNeedTDZ.add(entry.key.get());
    28672874        }
    28682875    }
     2876
     2877    m_cachedVariablesUnderTDZ = m_vm->m_compactVariableMap->get(environment);
     2878    return m_cachedVariablesUnderTDZ;
    28692879}
    28702880
     
    28772887{
    28782888    m_TDZStack = preservedStack.m_preservedTDZStack;
     2889    m_cachedVariablesUnderTDZ = { };
    28792890}
    28802891
  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r240981 r241571  
    11381138            }
    11391139
    1140             VariableEnvironment variablesUnderTDZ;
    1141             getVariablesUnderTDZ(variablesUnderTDZ);
     1140            CompactVariableMap::Handle variablesUnderTDZ = getVariablesUnderTDZ();
    11421141
    11431142            // FIXME: These flags, ParserModes and propagation to XXXCodeBlocks should be reorganized.
     
    11481147                constructAbility = ConstructAbility::CanConstruct;
    11491148
    1150             return UnlinkedFunctionExecutable::create(m_vm, m_scopeNode->source(), metadata, isBuiltinFunction() ? UnlinkedBuiltinFunction : UnlinkedNormalFunction, constructAbility, scriptMode(), variablesUnderTDZ, newDerivedContextType);
    1151         }
    1152 
    1153         void getVariablesUnderTDZ(VariableEnvironment&);
     1149            return UnlinkedFunctionExecutable::create(m_vm, m_scopeNode->source(), metadata, isBuiltinFunction() ? UnlinkedBuiltinFunction : UnlinkedNormalFunction, constructAbility, scriptMode(), WTFMove(variablesUnderTDZ), newDerivedContextType);
     1150        }
     1151
     1152        CompactVariableMap::Handle getVariablesUnderTDZ();
    11541153
    11551154        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);
     
    13141313        DerivedContextType m_derivedContextType { DerivedContextType::None };
    13151314
     1315        CompactVariableMap::Handle m_cachedVariablesUnderTDZ;
     1316
    13161317        using CatchEntry = std::tuple<TryData*, VirtualRegister, VirtualRegister>;
    13171318        Vector<CatchEntry> m_catchesToEmit;
  • trunk/Source/JavaScriptCore/parser/VariableEnvironment.cpp

    r239755 r241571  
    184184}
    185185
     186CompactVariableMap::Handle::Handle(const CompactVariableMap::Handle& other)
     187{
     188    *this = other;
     189}
     190
     191CompactVariableMap::Handle& CompactVariableMap::Handle::operator=(const Handle& other)
     192{
     193    m_map = other.m_map;
     194    m_environment = other.m_environment;
     195
     196    if (!m_map) {
     197        ASSERT(!m_environment);
     198        // This happens if `other` were moved into a different handle.
     199        return *this;
     200    }
     201
     202    auto iter = m_map->m_map.find(CompactVariableMapKey { *m_environment });
     203    RELEASE_ASSERT(iter != m_map->m_map.end());
     204    ++iter->value;
     205
     206    return *this;
     207}
     208
    186209} // namespace JSC
  • trunk/Source/JavaScriptCore/parser/VariableEnvironment.h

    r240255 r241571  
    205205public:
    206206    class Handle {
    207         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.
    208207    public:
     208        Handle() = default;
     209
    209210        Handle(CompactVariableEnvironment& environment, CompactVariableMap& map)
    210211            : m_environment(&environment)
     
    219220            other.m_environment = nullptr;
    220221        }
     222
     223        Handle(const Handle&);
     224        Handle& operator=(const Handle&);
     225
    221226        ~Handle();
     227
     228        explicit operator bool() const { return !!m_map; }
    222229
    223230        const CompactVariableEnvironment& environment() const
     
    227234
    228235    private:
    229         CompactVariableEnvironment* m_environment;
     236        CompactVariableEnvironment* m_environment { nullptr };
    230237        RefPtr<CompactVariableMap> m_map;
    231238    };
  • trunk/Source/JavaScriptCore/runtime/CodeCache.cpp

    r241552 r241571  
    148148    VariableEnvironment emptyTDZVariables;
    149149    ConstructAbility constructAbility = constructAbilityForParseMode(metadata->parseMode());
    150     UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, metadata, UnlinkedNormalFunction, constructAbility, JSParserScriptMode::Classic, emptyTDZVariables, DerivedContextType::None);
     150    UnlinkedFunctionExecutable* functionExecutable = UnlinkedFunctionExecutable::create(&vm, source, metadata, UnlinkedNormalFunction, constructAbility, JSParserScriptMode::Classic, vm.m_compactVariableMap->get(emptyTDZVariables), DerivedContextType::None);
    151151
    152152    functionExecutable->setSourceURLDirective(source.provider()->sourceURL());
Note: See TracChangeset for help on using the changeset viewer.