Attachment 362435 Details for Bug 194706 – Patch

[patch] Patch

bug-194706-20190219234846.patch (text/plain), 10.36 KB, created by Tadeu Zagallo on 2019-02-19 14:49:23 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-02-19 14:49:23 PST

Size: 10.36 KB

patch

obsolete

>Subversion Revision: 241760
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 6c2f02508f0426495ee4591332883cb4c6ba02d3..554c95fe25eb772c1ca2cc6300fc0cce0d8465ba 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-02-19  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Cache CompactVariableMap::Handle instead of VariableEnvironment for UnlinkedFunctionExecutable
>+        https://bugs.webkit.org/show_bug.cgi?id=194706
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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.
>+
>+        * 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):
>+
> 2019-02-19  Tadeu Zagallo  <tzagallo@apple.com>
> 
>         Move bytecode cache-related filesystem code out of CodeCache
>diff --git a/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h b/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h
>index defd2f08b87181e6b7f4ff60156907fdb6eb397f..b0c305e2f5b9d0b338581abd69f08de87753fe04 100644
>--- a/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h
>+++ b/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h
>@@ -191,7 +191,7 @@ public:
> 
> private:
>     UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, CompactVariableMap::Handle,  JSC::DerivedContextType, bool isBuiltinDefaultClassConstructor);
>-    UnlinkedFunctionExecutable(Decoder&, VariableEnvironment&, const CachedFunctionExecutable&);
>+    UnlinkedFunctionExecutable(Decoder&, CompactVariableMap::Handle, const CachedFunctionExecutable&);
> 
>     unsigned m_firstLineOffset;
>     unsigned m_lineCount;
>diff --git a/Source/JavaScriptCore/parser/VariableEnvironment.cpp b/Source/JavaScriptCore/parser/VariableEnvironment.cpp
>index a93ae7adbdfe6686c93b7fc1850737d9e1f0f1bb..76e0c288c652e116e140bd06797b83c24d172ece 100644
>--- a/Source/JavaScriptCore/parser/VariableEnvironment.cpp
>+++ b/Source/JavaScriptCore/parser/VariableEnvironment.cpp
>@@ -153,13 +153,18 @@ VariableEnvironment CompactVariableEnvironment::toVariableEnvironment() const
> 
> CompactVariableMap::Handle CompactVariableMap::get(const VariableEnvironment& env)
> {
>-    auto* environment = new CompactVariableEnvironment(env);
>+    return get(new CompactVariableEnvironment(env));
>+}
>+
>+CompactVariableMap::Handle CompactVariableMap::get(CompactVariableEnvironment* environment, DeleteUnusedEnvironment deleteUnusedEnvironment)
>+{
>     CompactVariableMapKey key { *environment };
>     auto addResult = m_map.add(key, 1);
>     if (addResult.isNewEntry)
>         return CompactVariableMap::Handle(*environment, *this);
> 
>-    delete environment;
>+    if (deleteUnusedEnvironment)
>+        delete environment;
>     ++addResult.iterator->value;
>     return CompactVariableMap::Handle(addResult.iterator->key.environment(), *this);
> }
>diff --git a/Source/JavaScriptCore/parser/VariableEnvironment.h b/Source/JavaScriptCore/parser/VariableEnvironment.h
>index ee6cce6e786ca56aa00081d095f0264bc3cbee5a..02800d69c629627aa72fa4e4cc267f768515154a 100644
>--- a/Source/JavaScriptCore/parser/VariableEnvironment.h
>+++ b/Source/JavaScriptCore/parser/VariableEnvironment.h
>@@ -125,6 +125,9 @@ private:
> class CompactVariableEnvironment {
>     WTF_MAKE_FAST_ALLOCATED;
>     WTF_MAKE_NONCOPYABLE(CompactVariableEnvironment);
>+
>+    friend class CachedCompactVariableEnvironment;
>+
> public:
>     CompactVariableEnvironment(const VariableEnvironment&);
>     VariableEnvironment toVariableEnvironment() const;
>@@ -133,6 +136,8 @@ public:
>     unsigned hash() const { return m_hash; }
> 
> private:
>+    CompactVariableEnvironment() = default;
>+
>     Vector<RefPtr<UniquedStringImpl>> m_variables;
>     Vector<VariableEnvironmentEntry> m_variableMetadata;
>     unsigned m_hash;
>@@ -204,6 +209,8 @@ namespace JSC {
> class CompactVariableMap : public RefCounted<CompactVariableMap> {
> public:
>     class Handle {
>+        friend class CachedCompactVariableMapHandle;
>+
>     public:
>         Handle() = default;
> 
>@@ -241,6 +248,12 @@ public:
> 
> private:
>     friend class Handle;
>+    friend class CachedCompactVariableMapHandle;
>+
>+    enum DeleteUnusedEnvironment { No, Yes };
>+
>+    Handle get(CompactVariableEnvironment*, DeleteUnusedEnvironment = Yes);
>+
>     HashMap<CompactVariableMapKey, unsigned> m_map;
> };
> 
>diff --git a/Source/JavaScriptCore/runtime/CachedTypes.cpp b/Source/JavaScriptCore/runtime/CachedTypes.cpp
>index acd95dfa9650ed01346215da43799285cf4662f8..780f3e2cb64ea43a366fae5e23a268756fa1aec9 100644
>--- a/Source/JavaScriptCore/runtime/CachedTypes.cpp
>+++ b/Source/JavaScriptCore/runtime/CachedTypes.cpp
>@@ -878,6 +878,54 @@ private:
>     CachedHashMap<CachedRefPtr<CachedUniquedStringImpl>, VariableEnvironmentEntry, IdentifierRepHash, HashTraits<RefPtr<UniquedStringImpl>>, VariableEnvironmentEntryHashTraits> m_map;
> };
> 
>+class CachedCompactVariableEnvironment : public CachedObject<CompactVariableEnvironment> {
>+public:
>+    void encode(Encoder& encoder, const CompactVariableEnvironment& env)
>+    {
>+        m_variables.encode(encoder, env.m_variables);
>+        m_variableMetadata.encode(encoder, env.m_variableMetadata);
>+        m_hash = env.m_hash;
>+        m_isEverythingCaptured = env.m_isEverythingCaptured;
>+    }
>+
>+    void decode(Decoder& decoder, CompactVariableEnvironment& env) const
>+    {
>+        m_variables.decode(decoder, env.m_variables);
>+        m_variableMetadata.decode(decoder, env.m_variableMetadata);
>+        env.m_hash = m_hash;
>+        env.m_isEverythingCaptured = m_isEverythingCaptured;
>+    }
>+
>+    CompactVariableEnvironment* decode(Decoder& decoder) const
>+    {
>+        CompactVariableEnvironment* env = new CompactVariableEnvironment;
>+        decode(decoder, *env);
>+        return env;
>+    }
>+
>+private:
>+    CachedVector<CachedRefPtr<CachedUniquedStringImpl>> m_variables;
>+    CachedVector<VariableEnvironmentEntry> m_variableMetadata;
>+    unsigned m_hash;
>+    bool m_isEverythingCaptured;
>+};
>+
>+class CachedCompactVariableMapHandle : public CachedObject<CompactVariableMap::Handle> {
>+public:
>+    void encode(Encoder& encoder, const CompactVariableMap::Handle& handle)
>+    {
>+        m_environment.encode(encoder, handle.m_environment);
>+    }
>+
>+    CompactVariableMap::Handle decode(Decoder& decoder) const
>+    {
>+        return decoder.vm().m_compactVariableMap->get(m_environment.decode(decoder), CompactVariableMap::DeleteUnusedEnvironment::No);
>+    }
>+
>+private:
>+    CachedPtr<CachedCompactVariableEnvironment> m_environment;
>+};
>+
> template<typename T, typename Source = SourceType<T>>
> class CachedArray : public VariableLengthObject<Source*> {
> public:
>@@ -1517,7 +1565,7 @@ private:
>     CachedIdentifier m_ecmaName;
>     CachedIdentifier m_inferredName;
> 
>-    CachedVariableEnvironment m_parentScopeTDZVariables;
>+    CachedCompactVariableMapHandle m_parentScopeTDZVariables;
> 
>     CachedWriteBarrier<CachedFunctionCodeBlock, UnlinkedFunctionCodeBlock> m_unlinkedCodeBlockForCall;
>     CachedWriteBarrier<CachedFunctionCodeBlock, UnlinkedFunctionCodeBlock> m_unlinkedCodeBlockForConstruct;
>@@ -1852,7 +1900,7 @@ ALWAYS_INLINE void CachedFunctionExecutable::encode(Encoder& encoder, const Unli
>     m_ecmaName.encode(encoder, executable.ecmaName());
>     m_inferredName.encode(encoder, executable.inferredName());
> 
>-    m_parentScopeTDZVariables.encode(encoder, executable.parentScopeTDZVariables());
>+    m_parentScopeTDZVariables.encode(encoder, executable.m_parentScopeTDZVariables);
> 
>     m_unlinkedCodeBlockForCall.encode(encoder, executable.m_unlinkedCodeBlockForCall);
>     m_unlinkedCodeBlockForConstruct.encode(encoder, executable.m_unlinkedCodeBlockForConstruct);
>@@ -1860,10 +1908,8 @@ ALWAYS_INLINE void CachedFunctionExecutable::encode(Encoder& encoder, const Unli
> 
> ALWAYS_INLINE UnlinkedFunctionExecutable* CachedFunctionExecutable::decode(Decoder& decoder) const
> {
>-    VariableEnvironment env;
>-    m_parentScopeTDZVariables.decode(decoder, env);
>-
>-    UnlinkedFunctionExecutable* executable = new (NotNull, allocateCell<UnlinkedFunctionExecutable>(decoder.vm().heap)) UnlinkedFunctionExecutable(decoder, env, *this);
>+    CompactVariableMap::Handle env = m_parentScopeTDZVariables.decode(decoder);
>+    UnlinkedFunctionExecutable* executable = new (NotNull, allocateCell<UnlinkedFunctionExecutable>(decoder.vm().heap)) UnlinkedFunctionExecutable(decoder, WTFMove(env), *this);
>     executable->finishCreation(decoder.vm());
> 
>     m_unlinkedCodeBlockForCall.decode(decoder, executable->m_unlinkedCodeBlockForCall, executable);
>@@ -1872,7 +1918,7 @@ ALWAYS_INLINE UnlinkedFunctionExecutable* CachedFunctionExecutable::decode(Decod
>     return executable;
> }
> 
>-ALWAYS_INLINE UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(Decoder& decoder, VariableEnvironment& parentScopeTDZVariables, const CachedFunctionExecutable& cachedExecutable)
>+ALWAYS_INLINE UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(Decoder& decoder, CompactVariableMap::Handle parentScopeTDZVariables, const CachedFunctionExecutable& cachedExecutable)
>     : Base(decoder.vm(), decoder.vm().unlinkedFunctionExecutableStructure.get())
>     , m_firstLineOffset(cachedExecutable.firstLineOffset())
>     , m_lineCount(cachedExecutable.lineCount())
>@@ -1902,7 +1948,7 @@ ALWAYS_INLINE UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(Decoder& de
>     , m_ecmaName(cachedExecutable.ecmaName(decoder))
>     , m_inferredName(cachedExecutable.inferredName(decoder))
> 
>-    , m_parentScopeTDZVariables(decoder.vm().m_compactVariableMap->get(parentScopeTDZVariables))
>+    , m_parentScopeTDZVariables(WTFMove(parentScopeTDZVariables))
> 
>     , m_rareData(cachedExecutable.rareData(decoder))
> {

Actions: View | Formatted Diff | Diff

Attachments on bug 194706: 362118 | 362435 | 362485 | 362498 | 362700 | 362702