Changeset 241938 in webkit


Ignore:
Timestamp:
Feb 22, 2019 1:08:35 AM (5 years ago)
Author:
Tadeu Zagallo
Message:

Cache CompactVariableMap::Handle instead of VariableEnvironment for UnlinkedFunctionExecutable
https://bugs.webkit.org/show_bug.cgi?id=194706

Reviewed by Saam Barati.

In https://bugs.webkit.org/show_bug.cgi?id=194583 we started using a
CompactVariableMap::Handle instead of VariableEnvironment for
UnlinkedFunctionExecutables, but we were creating the full environment
to encode the executable in the bytecode cache. This patch changes it so
that we cache the handle instead of the environment. This avoids duplicating
the VariableEnvironment whenever we have to cache two handles that point
to the environment.

  • bytecode/UnlinkedFunctionExecutable.h:
  • parser/VariableEnvironment.cpp:

(JSC::CompactVariableMap::get):

  • parser/VariableEnvironment.h:
  • runtime/CachedTypes.cpp:

(JSC::CachedCompactVariableEnvironment::encode):
(JSC::CachedCompactVariableEnvironment::decode const):
(JSC::CachedCompactVariableMapHandle::encode):
(JSC::CachedCompactVariableMapHandle::decode const):
(JSC::CachedFunctionExecutable::encode):
(JSC::CachedFunctionExecutable::decode const):
(JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):

Location:
trunk/Source/JavaScriptCore
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r241929 r241938  
     12019-02-22  Tadeu Zagallo  <tzagallo@apple.com>
     2
     3        Cache CompactVariableMap::Handle instead of VariableEnvironment for UnlinkedFunctionExecutable
     4        https://bugs.webkit.org/show_bug.cgi?id=194706
     5
     6        Reviewed by Saam Barati.
     7
     8        In https://bugs.webkit.org/show_bug.cgi?id=194583 we started using a
     9        CompactVariableMap::Handle instead of VariableEnvironment for
     10        UnlinkedFunctionExecutables, but we were creating the full environment
     11        to encode the executable in the bytecode cache. This patch changes it so
     12        that we cache the handle instead of the environment. This avoids duplicating
     13        the VariableEnvironment whenever we have to cache two handles that point
     14        to the environment.
     15
     16        * bytecode/UnlinkedFunctionExecutable.h:
     17        * parser/VariableEnvironment.cpp:
     18        (JSC::CompactVariableMap::get):
     19        * parser/VariableEnvironment.h:
     20        * runtime/CachedTypes.cpp:
     21        (JSC::CachedCompactVariableEnvironment::encode):
     22        (JSC::CachedCompactVariableEnvironment::decode const):
     23        (JSC::CachedCompactVariableMapHandle::encode):
     24        (JSC::CachedCompactVariableMapHandle::decode const):
     25        (JSC::CachedFunctionExecutable::encode):
     26        (JSC::CachedFunctionExecutable::decode const):
     27        (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
     28
    1292019-02-21  Saam Barati  <sbarati@apple.com>
    230
  • trunk/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h

    r241645 r241938  
    192192private:
    193193    UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, CompactVariableMap::Handle,  JSC::DerivedContextType, bool isBuiltinDefaultClassConstructor);
    194     UnlinkedFunctionExecutable(Decoder&, VariableEnvironment&, const CachedFunctionExecutable&);
     194    UnlinkedFunctionExecutable(Decoder&, CompactVariableMap::Handle, const CachedFunctionExecutable&);
    195195
    196196    unsigned m_firstLineOffset;
  • trunk/Source/JavaScriptCore/parser/VariableEnvironment.cpp

    r241571 r241938  
    155155{
    156156    auto* environment = new CompactVariableEnvironment(env);
     157    bool isNewEntry;
     158    auto handle = get(environment, isNewEntry);
     159    if (!isNewEntry)
     160        delete environment;
     161    return handle;
     162}
     163
     164CompactVariableMap::Handle CompactVariableMap::get(CompactVariableEnvironment* environment, bool& isNewEntry)
     165{
    157166    CompactVariableMapKey key { *environment };
    158167    auto addResult = m_map.add(key, 1);
     168    isNewEntry = addResult.isNewEntry;
    159169    if (addResult.isNewEntry)
    160170        return CompactVariableMap::Handle(*environment, *this);
    161171
    162     delete environment;
    163172    ++addResult.iterator->value;
    164173    return CompactVariableMap::Handle(addResult.iterator->key.environment(), *this);
  • trunk/Source/JavaScriptCore/parser/VariableEnvironment.h

    r241571 r241938  
    126126    WTF_MAKE_FAST_ALLOCATED;
    127127    WTF_MAKE_NONCOPYABLE(CompactVariableEnvironment);
     128
     129    friend class CachedCompactVariableEnvironment;
     130
    128131public:
    129132    CompactVariableEnvironment(const VariableEnvironment&);
     
    134137
    135138private:
     139    CompactVariableEnvironment() = default;
     140
    136141    Vector<RefPtr<UniquedStringImpl>> m_variables;
    137142    Vector<VariableEnvironmentEntry> m_variableMetadata;
     
    205210public:
    206211    class Handle {
     212        friend class CachedCompactVariableMapHandle;
     213
    207214    public:
    208215        Handle() = default;
     
    242249private:
    243250    friend class Handle;
     251    friend class CachedCompactVariableMapHandle;
     252
     253    Handle get(CompactVariableEnvironment*, bool& isNewEntry);
     254
    244255    HashMap<CompactVariableMapKey, unsigned> m_map;
    245256};
  • trunk/Source/JavaScriptCore/runtime/CachedTypes.cpp

    r241733 r241938  
    128128    }
    129129
    130     WTF::Optional<ptrdiff_t> offsetForPtr(const void* ptr)
     130    WTF::Optional<ptrdiff_t> cachedOffsetForPtr(const void* ptr)
    131131    {
    132132        auto it = m_ptrToOffsetMap.find(ptr);
     
    228228    ~Decoder()
    229229    {
    230         for (auto& pair : m_finalizers)
    231             pair.value();
     230        for (auto& finalizer : m_finalizers)
     231            finalizer();
    232232    }
    233233
     
    246246    }
    247247
    248     WTF::Optional<void*> ptrForOffset(ptrdiff_t offset)
     248    WTF::Optional<void*> cachedPtrForOffset(ptrdiff_t offset)
    249249    {
    250250        auto it = m_offsetToPtrMap.find(offset);
     
    255255
    256256    template<typename Functor>
    257     void addFinalizer(ptrdiff_t offset, const Functor& fn)
    258     {
    259         m_finalizers.add(offset, fn);
     257    void addFinalizer(const Functor& finalizer)
     258    {
     259        m_finalizers.append(finalizer);
    260260    }
    261261
     
    267267#endif
    268268    HashMap<ptrdiff_t, void*> m_offsetToPtrMap;
    269     HashMap<ptrdiff_t, std::function<void()>> m_finalizers;
     269    Vector<std::function<void()>> m_finalizers;
    270270};
    271271
     
    378378            return;
    379379
    380         if (WTF::Optional<ptrdiff_t> offset = encoder.offsetForPtr(src)) {
     380        if (WTF::Optional<ptrdiff_t> offset = encoder.cachedOffsetForPtr(src)) {
    381381            this->m_offset = *offset - encoder.offsetOf(&this->m_offset);
    382382            return;
     
    389389
    390390    template<typename... Args>
    391     Source* decode(Decoder& decoder, Args... args) const
    392     {
    393         if (m_isEmpty)
     391    Source* decode(Decoder& decoder, bool& isNewAllocation, Args&&... args) const
     392    {
     393        if (m_isEmpty) {
     394            isNewAllocation = false;
    394395            return nullptr;
     396        }
    395397
    396398        ptrdiff_t bufferOffset = decoder.offsetOf(this->buffer());
    397         if (WTF::Optional<void*> ptr = decoder.ptrForOffset(bufferOffset))
     399        if (WTF::Optional<void*> ptr = decoder.cachedPtrForOffset(bufferOffset)) {
     400            isNewAllocation = false;
    398401            return reinterpret_cast<Source*>(*ptr);
    399 
    400         Source* ptr = get()->decode(decoder, args...);
     402        }
     403
     404        isNewAllocation = true;
     405        Source* ptr = get()->decode(decoder, std::forward<Args>(args)...);
    401406        decoder.cacheOffset(bufferOffset, ptr);
    402407        return ptr;
     408    }
     409
     410    template<typename... Args>
     411    Source* decode(Decoder& decoder, Args&&... args) const
     412    {
     413        bool unusedIsNewAllocation;
     414        return decode(decoder, unusedIsNewAllocation, std::forward<Args>(args)...);
    403415    }
    404416
     
    432444    RefPtr<Source> decode(Decoder& decoder) const
    433445    {
    434         Source* decodedPtr = m_ptr.decode(decoder);
     446        bool isNewAllocation;
     447        Source* decodedPtr = m_ptr.decode(decoder, isNewAllocation);
    435448        if (!decodedPtr)
    436449            return nullptr;
    437         decoder.addFinalizer(decoder.offsetOf(m_ptr.buffer()), [=] { derefIfNotNull(decodedPtr); });
     450        if (isNewAllocation) {
     451            decoder.addFinalizer([=] {
     452                derefIfNotNull(decodedPtr);
     453            });
     454        }
    438455        refIfNotNull(decodedPtr);
    439456        return adoptRef(decodedPtr);
     
    877894    bool m_isEverythingCaptured;
    878895    CachedHashMap<CachedRefPtr<CachedUniquedStringImpl>, VariableEnvironmentEntry, IdentifierRepHash, HashTraits<RefPtr<UniquedStringImpl>>, VariableEnvironmentEntryHashTraits> m_map;
     896};
     897
     898class CachedCompactVariableEnvironment : public CachedObject<CompactVariableEnvironment> {
     899public:
     900    void encode(Encoder& encoder, const CompactVariableEnvironment& env)
     901    {
     902        m_variables.encode(encoder, env.m_variables);
     903        m_variableMetadata.encode(encoder, env.m_variableMetadata);
     904        m_hash = env.m_hash;
     905        m_isEverythingCaptured = env.m_isEverythingCaptured;
     906    }
     907
     908    void decode(Decoder& decoder, CompactVariableEnvironment& env) const
     909    {
     910        m_variables.decode(decoder, env.m_variables);
     911        m_variableMetadata.decode(decoder, env.m_variableMetadata);
     912        env.m_hash = m_hash;
     913        env.m_isEverythingCaptured = m_isEverythingCaptured;
     914    }
     915
     916    CompactVariableEnvironment* decode(Decoder& decoder) const
     917    {
     918        CompactVariableEnvironment* env = new CompactVariableEnvironment;
     919        decode(decoder, *env);
     920        return env;
     921    }
     922
     923private:
     924    CachedVector<CachedRefPtr<CachedUniquedStringImpl>> m_variables;
     925    CachedVector<VariableEnvironmentEntry> m_variableMetadata;
     926    unsigned m_hash;
     927    bool m_isEverythingCaptured;
     928};
     929
     930class CachedCompactVariableMapHandle : public CachedObject<CompactVariableMap::Handle> {
     931public:
     932    void encode(Encoder& encoder, const CompactVariableMap::Handle& handle)
     933    {
     934        m_environment.encode(encoder, handle.m_environment);
     935    }
     936
     937    CompactVariableMap::Handle decode(Decoder& decoder) const
     938    {
     939        bool isNewAllocation;
     940        CompactVariableEnvironment* environment = m_environment.decode(decoder, isNewAllocation);
     941        bool isNewEntry;
     942        CompactVariableMap::Handle handle = decoder.vm().m_compactVariableMap->get(environment, isNewEntry);
     943        if (!isNewAllocation)
     944            ASSERT(!isNewEntry);
     945        else if (!isNewEntry) {
     946            decoder.addFinalizer([=] {
     947                delete environment;
     948            });
     949        }
     950        return handle;
     951    }
     952
     953private:
     954    CachedPtr<CachedCompactVariableEnvironment> m_environment;
    879955};
    880956
     
    15181594    CachedIdentifier m_inferredName;
    15191595
    1520     CachedVariableEnvironment m_parentScopeTDZVariables;
     1596    CachedCompactVariableMapHandle m_parentScopeTDZVariables;
    15211597
    15221598    CachedWriteBarrier<CachedFunctionCodeBlock, UnlinkedFunctionCodeBlock> m_unlinkedCodeBlockForCall;
     
    18531929    m_inferredName.encode(encoder, executable.inferredName());
    18541930
    1855     m_parentScopeTDZVariables.encode(encoder, executable.parentScopeTDZVariables());
     1931    m_parentScopeTDZVariables.encode(encoder, executable.m_parentScopeTDZVariables);
    18561932
    18571933    m_unlinkedCodeBlockForCall.encode(encoder, executable.m_unlinkedCodeBlockForCall);
     
    18611937ALWAYS_INLINE UnlinkedFunctionExecutable* CachedFunctionExecutable::decode(Decoder& decoder) const
    18621938{
    1863     VariableEnvironment env;
    1864     m_parentScopeTDZVariables.decode(decoder, env);
    1865 
    1866     UnlinkedFunctionExecutable* executable = new (NotNull, allocateCell<UnlinkedFunctionExecutable>(decoder.vm().heap)) UnlinkedFunctionExecutable(decoder, env, *this);
     1939    CompactVariableMap::Handle env = m_parentScopeTDZVariables.decode(decoder);
     1940    UnlinkedFunctionExecutable* executable = new (NotNull, allocateCell<UnlinkedFunctionExecutable>(decoder.vm().heap)) UnlinkedFunctionExecutable(decoder, WTFMove(env), *this);
    18671941    executable->finishCreation(decoder.vm());
    18681942
     
    18731947}
    18741948
    1875 ALWAYS_INLINE UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(Decoder& decoder, VariableEnvironment& parentScopeTDZVariables, const CachedFunctionExecutable& cachedExecutable)
     1949ALWAYS_INLINE UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(Decoder& decoder, CompactVariableMap::Handle parentScopeTDZVariables, const CachedFunctionExecutable& cachedExecutable)
    18761950    : Base(decoder.vm(), decoder.vm().unlinkedFunctionExecutableStructure.get())
    18771951    , m_firstLineOffset(cachedExecutable.firstLineOffset())
     
    19031977    , m_inferredName(cachedExecutable.inferredName(decoder))
    19041978
    1905     , m_parentScopeTDZVariables(decoder.vm().m_compactVariableMap->get(parentScopeTDZVariables))
     1979    , m_parentScopeTDZVariables(WTFMove(parentScopeTDZVariables))
    19061980
    19071981    , m_rareData(cachedExecutable.rareData(decoder))
Note: See TracChangeset for help on using the changeset viewer.