Bytecode cache should not encode the SourceProvider for UnlinkedFunctionExecutable...
authortzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Apr 2019 22:23:38 +0000 (22:23 +0000)
committertzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 15 Apr 2019 22:23:38 +0000 (22:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196878

Reviewed by Saam Barati.

Every time we encode an (Unlinked)SourceCode, we encode its SourceProvider,
including the full source if it's a StringSourceProvider. This wasn't an issue,
since the SourceCode contains a RefPtr to the SourceProvider, and the Encoder
would avoid encoding the provider multiple times. With the addition of the
incremental cache, each UnlinkedFunctionCodeBlock is encoded in isolation, which
means we can no longer deduplicate it and the full program text was being encoded
multiple times in the cache.
As a work around, this patch adds a custom cached type for encoding the SourceCode
without its provider, and later injects the SourceProvider through the Decoder.

* parser/SourceCode.h:
* parser/UnlinkedSourceCode.h:
(JSC::UnlinkedSourceCode::provider const):
* runtime/CachedTypes.cpp:
(JSC::Decoder::Decoder):
(JSC::Decoder::create):
(JSC::Decoder::provider const):
(JSC::CachedSourceCodeWithoutProvider::encode):
(JSC::CachedSourceCodeWithoutProvider::decode const):
(JSC::decodeCodeBlockImpl):
* runtime/CachedTypes.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244300 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/parser/SourceCode.h
Source/JavaScriptCore/parser/UnlinkedSourceCode.h
Source/JavaScriptCore/runtime/CachedTypes.cpp
Source/JavaScriptCore/runtime/CachedTypes.h

index 7a02fdc..50da1dc 100644 (file)
@@ -1,3 +1,32 @@
+2019-04-15  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Bytecode cache should not encode the SourceProvider for UnlinkedFunctionExecutable's classSource
+        https://bugs.webkit.org/show_bug.cgi?id=196878
+
+        Reviewed by Saam Barati.
+
+        Every time we encode an (Unlinked)SourceCode, we encode its SourceProvider,
+        including the full source if it's a StringSourceProvider. This wasn't an issue,
+        since the SourceCode contains a RefPtr to the SourceProvider, and the Encoder
+        would avoid encoding the provider multiple times. With the addition of the
+        incremental cache, each UnlinkedFunctionCodeBlock is encoded in isolation, which
+        means we can no longer deduplicate it and the full program text was being encoded
+        multiple times in the cache.
+        As a work around, this patch adds a custom cached type for encoding the SourceCode
+        without its provider, and later injects the SourceProvider through the Decoder.
+
+        * parser/SourceCode.h:
+        * parser/UnlinkedSourceCode.h:
+        (JSC::UnlinkedSourceCode::provider const):
+        * runtime/CachedTypes.cpp:
+        (JSC::Decoder::Decoder):
+        (JSC::Decoder::create):
+        (JSC::Decoder::provider const):
+        (JSC::CachedSourceCodeWithoutProvider::encode):
+        (JSC::CachedSourceCodeWithoutProvider::decode const):
+        (JSC::decodeCodeBlockImpl):
+        * runtime/CachedTypes.h:
+
 2019-04-15  Robin Morisset  <rmorisset@apple.com>
 
         MarkedSpace.cpp is not in the Xcode workspace
index fbe4c9a..aad287d 100644 (file)
@@ -34,6 +34,7 @@ namespace JSC {
 
     class SourceCode : public UnlinkedSourceCode {
         friend class CachedSourceCode;
+        friend class CachedSourceCodeWithoutProvider;
 
     public:
         SourceCode()
index f7b34f9..bb774a9 100644 (file)
@@ -36,6 +36,7 @@ namespace JSC {
     class UnlinkedSourceCode {
         template<typename SourceType>
         friend class CachedUnlinkedSourceCodeShape;
+        friend class CachedSourceCodeWithoutProvider;
 
     public:
         UnlinkedSourceCode()
@@ -73,7 +74,7 @@ namespace JSC {
 
         bool isHashTableDeletedValue() const { return m_provider.isHashTableDeletedValue(); }
 
-        const SourceProvider& provider() const
+        SourceProvider& provider() const
         {
             return *m_provider;
         }
index 57bc7b3..579ad3d 100644 (file)
@@ -220,9 +220,10 @@ private:
     LeafExecutableMap m_leafExecutables;
 };
 
-Decoder::Decoder(VM& vm, Ref<CachedBytecode> cachedBytecode)
+Decoder::Decoder(VM& vm, Ref<CachedBytecode> cachedBytecode, RefPtr<SourceProvider> provider)
     : m_vm(vm)
     , m_cachedBytecode(WTFMove(cachedBytecode))
+    , m_provider(provider)
 {
 }
 
@@ -232,9 +233,9 @@ Decoder::~Decoder()
         finalizer();
 }
 
-Ref<Decoder> Decoder::create(VM& vm, Ref<CachedBytecode> cachedBytecode)
+Ref<Decoder> Decoder::create(VM& vm, Ref<CachedBytecode> cachedBytecode, RefPtr<SourceProvider> provider)
 {
-    return adoptRef(*new Decoder(vm, WTFMove(cachedBytecode)));
+    return adoptRef(*new Decoder(vm, WTFMove(cachedBytecode), WTFMove(provider)));
 }
 
 size_t Decoder::size() const
@@ -294,6 +295,11 @@ void Decoder::addFinalizer(const Functor& fn)
     m_finalizers.append(fn);
 }
 
+RefPtr<SourceProvider> Decoder::provider() const
+{
+    return m_provider;
+}
+
 template<typename T>
 static std::enable_if_t<std::is_same<T, SourceType<T>>::value> encode(Encoder&, T& dst, const SourceType<T>& src)
 {
@@ -1547,6 +1553,35 @@ private:
     int m_startColumn;
 };
 
+class CachedSourceCodeWithoutProvider : public CachedObject<SourceCode> {
+public:
+    void encode(Encoder&, const SourceCode& sourceCode)
+    {
+        m_hasProvider = !!sourceCode.provider();
+        m_startOffset = sourceCode.startOffset();
+        m_endOffset = sourceCode.endOffset();
+        m_firstLine = sourceCode.firstLine().zeroBasedInt();
+        m_startColumn = sourceCode.startColumn().zeroBasedInt();
+    }
+
+    void decode(Decoder& decoder, SourceCode& sourceCode) const
+    {
+        if (m_hasProvider)
+            sourceCode.m_provider = decoder.provider();
+        sourceCode.m_startOffset = m_startOffset;
+        sourceCode.m_endOffset = m_endOffset;
+        sourceCode.m_firstLine = OrdinalNumber::fromZeroBasedInt(m_firstLine);
+        sourceCode.m_startColumn = OrdinalNumber::fromZeroBasedInt(m_startColumn);
+    }
+
+private:
+    bool m_hasProvider;
+    int m_startOffset;
+    int m_endOffset;
+    int m_firstLine;
+    int m_startColumn;
+};
+
 class CachedFunctionExecutableRareData : public CachedObject<UnlinkedFunctionExecutable::RareData> {
 public:
     void encode(Encoder& encoder, const UnlinkedFunctionExecutable::RareData& rareData)
@@ -1565,7 +1600,7 @@ public:
     }
 
 private:
-    CachedSourceCode m_classSource;
+    CachedSourceCodeWithoutProvider m_classSource;
     CachedCompactVariableMapHandle m_parentScopeTDZVariables;
 };
 
@@ -2297,7 +2332,7 @@ Ref<CachedBytecode> encodeFunctionCodeBlock(VM& vm, const UnlinkedFunctionCodeBl
 UnlinkedCodeBlock* decodeCodeBlockImpl(VM& vm, const SourceCodeKey& key, Ref<CachedBytecode> cachedBytecode)
 {
     const auto* cachedEntry = bitwise_cast<const GenericCacheEntry*>(cachedBytecode->data());
-    Ref<Decoder> decoder = Decoder::create(vm, WTFMove(cachedBytecode));
+    Ref<Decoder> decoder = Decoder::create(vm, WTFMove(cachedBytecode), &key.source().provider());
     std::pair<SourceCodeKey, UnlinkedCodeBlock*> entry;
     {
         DeferGC deferGC(vm.heap);
index 2e04b10..9d00e17 100644 (file)
@@ -77,7 +77,7 @@ class Decoder : public RefCounted<Decoder> {
     WTF_MAKE_NONCOPYABLE(Decoder);
 
 public:
-    static Ref<Decoder> create(VM&, Ref<CachedBytecode>);
+    static Ref<Decoder> create(VM&, Ref<CachedBytecode>, RefPtr<SourceProvider> = nullptr);
 
     ~Decoder();
 
@@ -91,18 +91,20 @@ public:
     CompactVariableMap::Handle handleForEnvironment(CompactVariableEnvironment*) const;
     void setHandleForEnvironment(CompactVariableEnvironment*, const CompactVariableMap::Handle&);
     void addLeafExecutable(const UnlinkedFunctionExecutable*, ptrdiff_t);
+    RefPtr<SourceProvider> provider() const;
 
     template<typename Functor>
     void addFinalizer(const Functor&);
 
 private:
-    Decoder(VM&, Ref<CachedBytecode>);
+    Decoder(VM&, Ref<CachedBytecode>, RefPtr<SourceProvider>);
 
     VM& m_vm;
     Ref<CachedBytecode> m_cachedBytecode;
     HashMap<ptrdiff_t, void*> m_offsetToPtrMap;
     Vector<std::function<void()>> m_finalizers;
     HashMap<CompactVariableEnvironment*, CompactVariableMap::Handle> m_environmentToHandleMap;
+    RefPtr<SourceProvider> m_provider;
 };
 
 JS_EXPORT_PRIVATE Ref<CachedBytecode> encodeCodeBlock(VM&, const SourceCodeKey&, const UnlinkedCodeBlock*);