Attachment 371455 Details for Bug 198023 – Patch

[patch] Patch

bug-198023-20190605171441.patch (text/plain), 14.37 KB, created by Yusuke Suzuki on 2019-06-05 17:14:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-06-05 17:14:42 PDT

Size: 14.37 KB

patch

obsolete

>Subversion Revision: 246084
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 521baaaa39e8ed45945f2be0f4398681e8473d70..485a59dde045016219b97dcec394af659f17db01 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,28 @@
>+2019-06-05  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] UnlinkedCodeBlock should be eventually jettisoned too or cached
>+        https://bugs.webkit.org/show_bug.cgi?id=198023
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * bytecode/UnlinkedCodeBlock.cpp:
>+        (JSC::UnlinkedCodeBlock::UnlinkedCodeBlock):
>+        * bytecode/UnlinkedCodeBlock.h:
>+        (JSC::UnlinkedCodeBlock::usedInPreviousCycle const):
>+        (JSC::UnlinkedCodeBlock::setUsedInPreviousCycle):
>+        * bytecode/UnlinkedFunctionExecutable.cpp:
>+        (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
>+        (JSC::UnlinkedFunctionExecutable::visitChildren):
>+        (JSC::UnlinkedFunctionExecutable::unlinkedCodeBlockFor):
>+        (JSC::UnlinkedFunctionExecutable::decodeCachedCodeBlocks):
>+        (JSC::UnlinkedFunctionExecutable::finalizeUnconditionally):
>+        * bytecode/UnlinkedFunctionExecutable.h:
>+        * heap/Heap.cpp:
>+        (JSC::Heap::finalizeUnconditionalFinalizers):
>+        * runtime/CachedTypes.cpp:
>+        (JSC::UnlinkedCodeBlock::UnlinkedCodeBlock):
>+        (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
>+
> 2019-06-04  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         Unreviewed, update exception scope for putByIndexBeyondVectorLength
>diff --git a/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp b/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp
>index f9c3ba9d71647d423db18a14419804d34f30f441..437b3a823252a427cf3ab9a1eca2ec98d4ed84c7 100644
>--- a/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp
>@@ -71,6 +71,7 @@ UnlinkedCodeBlock::UnlinkedCodeBlock(VM* vm, Structure* structure, CodeType code
>     , m_evalContextType(static_cast<unsigned>(info.evalContextType()))
>     , m_codeType(static_cast<unsigned>(codeType))
>     , m_didOptimize(static_cast<unsigned>(MixedTriState))
>+    , m_usedInPreviousCycle(false)
>     , m_parseMode(info.parseMode())
>     , m_codeGenerationMode(codeGenerationMode)
>     , m_metadata(UnlinkedMetadataTable::create())
>diff --git a/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h b/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h
>index 2a350a1f11cf2131f72c2b4bd809018fe712eff2..61a76744033f35aa9c6c81363086054661549b8a 100644
>--- a/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.h
>@@ -370,6 +370,12 @@ class UnlinkedCodeBlock : public JSCell {
>         return m_metadata->sizeInBytes();
>     }
> 
>+    bool usedInPreviousCycle() const { return m_usedInPreviousCycle; }
>+
>+    void setUsedInPreviousCycle(bool used)
>+    {
>+        m_usedInPreviousCycle = used;
>+    }
> 
> protected:
>     UnlinkedCodeBlock(VM*, Structure*, CodeType, const ExecutableInfo&, OptionSet<CodeGenerationMode>);
>@@ -424,6 +430,7 @@ class UnlinkedCodeBlock : public JSCell {
>     unsigned m_evalContextType : 2;
>     unsigned m_codeType : 2;
>     unsigned m_didOptimize : 2;
>+    unsigned m_usedInPreviousCycle : 1;
> public:
>     ConcurrentJSLock m_lock;
> private:
>diff --git a/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp b/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp
>index 14a069c5e07608d309182293656eb2187fd97fd6..0312d59ec214ab8469ab69aa2f8460b458f64f6c 100644
>--- a/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp
>+++ b/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.cpp
>@@ -107,6 +107,7 @@ UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(VM* vm, Structure* struct
>     , m_constructorKind(static_cast<unsigned>(node->constructorKind()))
>     , m_functionMode(static_cast<unsigned>(node->functionMode()))
>     , m_derivedContextType(static_cast<unsigned>(derivedContextType))
>+    , m_isGeneratedFromCache(false)
>     , m_unlinkedCodeBlockForCall()
>     , m_unlinkedCodeBlockForConstruct()
>     , m_name(node->ident())
>@@ -142,7 +143,21 @@ void UnlinkedFunctionExecutable::visitChildren(JSCell* cell, SlotVisitor& visito
>     UnlinkedFunctionExecutable* thisObject = jsCast<UnlinkedFunctionExecutable*>(cell);
>     ASSERT_GC_OBJECT_INHERITS(thisObject, info());
>     Base::visitChildren(thisObject, visitor);
>-    if (!thisObject->m_isCached) {
>+
>+    // Currently, bytecode cache assumes that the tree of UnlinkedFunctionExecutable and UnlinkedCodeBlock will not be destroyed while the parent is live.
>+    // Bytecode cache uses this asumption to avoid duplicate materialization by bookkeeping the heap cells in the offste-to-pointer map.
>+    if (VM::isInMiniMode() && !thisObject->m_isGeneratedFromCache) {
>+        auto markIfProfitable = [&] (WriteBarrier<UnlinkedFunctionCodeBlock>& unlinkedCodeBlock) {
>+            if (!unlinkedCodeBlock)
>+                return;
>+            if (unlinkedCodeBlock->didOptimize() == TrueTriState)
>+                visitor.append(unlinkedCodeBlock);
>+            else if (unlinkedCodeBlock->usedInPreviousCycle())
>+                visitor.append(unlinkedCodeBlock);
>+        };
>+        markIfProfitable(thisObject->m_unlinkedCodeBlockForCall);
>+        markIfProfitable(thisObject->m_unlinkedCodeBlockForConstruct);
>+    } else if (!thisObject->m_isCached) {
>         visitor.append(thisObject->m_unlinkedCodeBlockForCall);
>         visitor.append(thisObject->m_unlinkedCodeBlockForConstruct);
>     }
>@@ -197,56 +212,49 @@ UnlinkedFunctionExecutable* UnlinkedFunctionExecutable::fromGlobalCode(
>     return executable;
> }
> 
>-UnlinkedFunctionCodeBlock* UnlinkedFunctionExecutable::unlinkedCodeBlockFor(CodeSpecializationKind specializationKind)
>-{
>-    switch (specializationKind) {
>-    case CodeForCall:
>-        return m_unlinkedCodeBlockForCall.get();
>-    case CodeForConstruct:
>-        return m_unlinkedCodeBlockForConstruct.get();
>-    }
>-    ASSERT_NOT_REACHED();
>-    return nullptr;
>-}
>-
> UnlinkedFunctionCodeBlock* UnlinkedFunctionExecutable::unlinkedCodeBlockFor(
>     VM& vm, const SourceCode& source, CodeSpecializationKind specializationKind, 
>     OptionSet<CodeGenerationMode> codeGenerationMode, ParserError& error, SourceParseMode parseMode)
> {
>-    if (m_isCached)
>-        decodeCachedCodeBlocks();
>-    switch (specializationKind) {
>-    case CodeForCall:
>-        if (UnlinkedFunctionCodeBlock* codeBlock = m_unlinkedCodeBlockForCall.get())
>-            return codeBlock;
>-        break;
>-    case CodeForConstruct:
>-        if (UnlinkedFunctionCodeBlock* codeBlock = m_unlinkedCodeBlockForConstruct.get())
>-            return codeBlock;
>-        break;
>-    }
>-
>-    UnlinkedFunctionCodeBlock* result = generateUnlinkedFunctionCodeBlock(
>-        vm, this, source, specializationKind, codeGenerationMode, 
>-        isBuiltinFunction() ? UnlinkedBuiltinFunction : UnlinkedNormalFunction, 
>-        error, parseMode);
>-    
>-    if (error.isValid())
>-        return nullptr;
>-
>-    switch (specializationKind) {
>-    case CodeForCall:
>-        m_unlinkedCodeBlockForCall.set(vm, this, result);
>-        break;
>-    case CodeForConstruct:
>-        m_unlinkedCodeBlockForConstruct.set(vm, this, result);
>-        break;
>-    }
>-    vm.unlinkedFunctionExecutableSpace.set.add(this);
>-    return result;
>+    UnlinkedFunctionCodeBlock* unlinkedCodeBlock = ([&] () -> UnlinkedFunctionCodeBlock* {
>+        if (m_isCached)
>+            decodeCachedCodeBlocks(vm);
>+        switch (specializationKind) {
>+        case CodeForCall:
>+            if (UnlinkedFunctionCodeBlock* codeBlock = m_unlinkedCodeBlockForCall.get())
>+                return codeBlock;
>+            break;
>+        case CodeForConstruct:
>+            if (UnlinkedFunctionCodeBlock* codeBlock = m_unlinkedCodeBlockForConstruct.get())
>+                return codeBlock;
>+            break;
>+        }
>+
>+        UnlinkedFunctionCodeBlock* result = generateUnlinkedFunctionCodeBlock(
>+            vm, this, source, specializationKind, codeGenerationMode, 
>+            isBuiltinFunction() ? UnlinkedBuiltinFunction : UnlinkedNormalFunction, 
>+            error, parseMode);
>+        
>+        if (error.isValid())
>+            return nullptr;
>+
>+        switch (specializationKind) {
>+        case CodeForCall:
>+            m_unlinkedCodeBlockForCall.set(vm, this, result);
>+            break;
>+        case CodeForConstruct:
>+            m_unlinkedCodeBlockForConstruct.set(vm, this, result);
>+            break;
>+        }
>+        vm.unlinkedFunctionExecutableSpace.set.add(this);
>+        return result;
>+    })();
>+    if (unlinkedCodeBlock)
>+        unlinkedCodeBlock->setUsedInPreviousCycle(true);
>+    return unlinkedCodeBlock;
> }
> 
>-void UnlinkedFunctionExecutable::decodeCachedCodeBlocks()
>+void UnlinkedFunctionExecutable::decodeCachedCodeBlocks(VM& vm)
> {
>     ASSERT(m_isCached);
>     ASSERT(m_decoder);
>@@ -256,7 +264,7 @@ void UnlinkedFunctionExecutable::decodeCachedCodeBlocks()
>     int32_t cachedCodeBlockForCallOffset = m_cachedCodeBlockForCallOffset;
>     int32_t cachedCodeBlockForConstructOffset = m_cachedCodeBlockForConstructOffset;
> 
>-    DeferGC deferGC(decoder->vm().heap);
>+    DeferGC deferGC(vm.heap);
> 
>     // No need to clear m_unlinkedCodeBlockForCall here, since we moved the decoder out of the same slot
>     if (cachedCodeBlockForCallOffset)
>@@ -268,7 +276,7 @@ void UnlinkedFunctionExecutable::decodeCachedCodeBlocks()
> 
>     WTF::storeStoreFence();
>     m_isCached = false;
>-    decoder->vm().heap.writeBarrier(this);
>+    vm.heap.writeBarrier(this);
> }
> 
> UnlinkedFunctionExecutable::RareData& UnlinkedFunctionExecutable::ensureRareDataSlow()
>@@ -284,4 +292,27 @@ void UnlinkedFunctionExecutable::setInvalidTypeProfilingOffsets()
>     m_typeProfilingEndOffset = std::numeric_limits<unsigned>::max();
> }
> 
>+void UnlinkedFunctionExecutable::finalizeUnconditionally(VM& vm)
>+{
>+    if (VM::isInMiniMode() && !m_isGeneratedFromCache) {
>+        bool isCleared = false;
>+        bool isStillValid = false;
>+        auto clearIfDead = [&] (WriteBarrier<UnlinkedFunctionCodeBlock>& unlinkedCodeBlock) {
>+            if (!unlinkedCodeBlock)
>+                return;
>+            if (!vm.heap.isMarked(unlinkedCodeBlock.get())) {
>+                unlinkedCodeBlock.clear();
>+                isCleared = true;
>+            } else {
>+                unlinkedCodeBlock->setUsedInPreviousCycle(false);
>+                isStillValid = true;
>+            }
>+        };
>+        clearIfDead(m_unlinkedCodeBlockForCall);
>+        clearIfDead(m_unlinkedCodeBlockForConstruct);
>+        if (isCleared && !isStillValid)
>+            vm.unlinkedFunctionExecutableSpace.set.remove(this);
>+    }
>+}
>+
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h b/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h
>index 0c7f89cd4eb024fcf80d78836c3f43c0a7cd23c3..bed95d309e7f16dcd69792ffed44a0d1e2dc8f11 100644
>--- a/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h
>+++ b/Source/JavaScriptCore/bytecode/UnlinkedFunctionExecutable.h
>@@ -114,8 +114,6 @@ class UnlinkedFunctionExecutable final : public JSCell {
>     unsigned typeProfilingEndOffset() const { return m_typeProfilingEndOffset; }
>     void setInvalidTypeProfilingOffsets();
> 
>-    UnlinkedFunctionCodeBlock* unlinkedCodeBlockFor(CodeSpecializationKind);
>-
>     UnlinkedFunctionCodeBlock* unlinkedCodeBlockFor(
>         VM&, const SourceCode&, CodeSpecializationKind, OptionSet<CodeGenerationMode>,
>         ParserError&, SourceParseMode);
>@@ -189,6 +187,8 @@ class UnlinkedFunctionExecutable final : public JSCell {
>         ensureRareData().m_sourceMappingURLDirective = sourceMappingURL;
>     }
> 
>+    void finalizeUnconditionally(VM&);
>+
>     struct RareData {
>         WTF_MAKE_STRUCT_FAST_ALLOCATED;
> 
>@@ -202,7 +202,7 @@ class UnlinkedFunctionExecutable final : public JSCell {
>     UnlinkedFunctionExecutable(VM*, Structure*, const SourceCode&, FunctionMetadataNode*, UnlinkedFunctionKind, ConstructAbility, JSParserScriptMode, Optional<CompactVariableMap::Handle>,  JSC::DerivedContextType, bool isBuiltinDefaultClassConstructor);
>     UnlinkedFunctionExecutable(Decoder&, const CachedFunctionExecutable&);
> 
>-    void decodeCachedCodeBlocks();
>+    void decodeCachedCodeBlocks(VM&);
> 
>     unsigned m_firstLineOffset : 31;
>     unsigned m_isInStrictContext : 1;
>@@ -228,6 +228,7 @@ class UnlinkedFunctionExecutable final : public JSCell {
>     unsigned m_constructorKind : 2;
>     unsigned m_functionMode : 2; // FunctionMode
>     unsigned m_derivedContextType: 2;
>+    unsigned m_isGeneratedFromCache : 1;
> 
>     union {
>         WriteBarrier<UnlinkedFunctionCodeBlock> m_unlinkedCodeBlockForCall;
>diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp
>index 6663b4f0d39ebbef9d270de03d2b5fc5e33d6482..a85207f12cf603f047881d164cc678274483ffee 100644
>--- a/Source/JavaScriptCore/heap/Heap.cpp
>+++ b/Source/JavaScriptCore/heap/Heap.cpp
>@@ -606,6 +606,7 @@ void Heap::finalizeUnconditionalFinalizers()
>         });
>     finalizeMarkedUnconditionalFinalizers<ExecutableToCodeBlockEdge>(vm()->executableToCodeBlockEdgesWithFinalizers);
>     finalizeMarkedUnconditionalFinalizers<StructureRareData>(vm()->structureRareDataSpace);
>+    finalizeMarkedUnconditionalFinalizers<UnlinkedFunctionExecutable>(vm()->unlinkedFunctionExecutableSpace.set);
>     if (vm()->m_weakSetSpace)
>         finalizeMarkedUnconditionalFinalizers<JSWeakSet>(*vm()->m_weakSetSpace);
>     if (vm()->m_weakMapSpace)
>diff --git a/Source/JavaScriptCore/runtime/CachedTypes.cpp b/Source/JavaScriptCore/runtime/CachedTypes.cpp
>index d461e569329f1d9e8b86ecbc357b9ae7699d718a..b7bc2a724c36ef81f8a7245cd476391091cb3130 100644
>--- a/Source/JavaScriptCore/runtime/CachedTypes.cpp
>+++ b/Source/JavaScriptCore/runtime/CachedTypes.cpp
>@@ -2032,6 +2032,8 @@ ALWAYS_INLINE UnlinkedCodeBlock::UnlinkedCodeBlock(Decoder& decoder, Structure*
> 
>     , m_didOptimize(static_cast<unsigned>(MixedTriState))
> 
>+    , m_usedInPreviousCycle(false)
>+
>     , m_features(cachedCodeBlock.features())
>     , m_parseMode(cachedCodeBlock.parseMode())
>     , m_codeGenerationMode(cachedCodeBlock.codeGenerationMode())
>@@ -2158,6 +2160,7 @@ ALWAYS_INLINE UnlinkedFunctionExecutable::UnlinkedFunctionExecutable(Decoder& de
>     , m_constructorKind(cachedExecutable.constructorKind())
>     , m_functionMode(cachedExecutable.functionMode())
>     , m_derivedContextType(cachedExecutable.derivedContextType())
>+    , m_isGeneratedFromCache(true)
>     , m_unlinkedCodeBlockForCall()
>     , m_unlinkedCodeBlockForConstruct()
>

Actions: View | Formatted Diff | Diff

Attachments on bug 198023: 370283 | 371361 | 371455 | 371472 | 371473 | 371474 | 371560 | 371564 | 371569 | 371624