Attachment 362820 Details for Bug 194975 – Patch

[patch] Patch

bug-194975-20190223011959.patch (text/plain), 16.94 KB, created by Yusuke Suzuki on 2019-02-23 01:20:00 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-02-23 01:20:00 PST

Size: 16.94 KB

patch

obsolete

>Subversion Revision: 241988
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 27ba48124e85c295fc7cf9b787511d7d1a4e6c9f..ed744205db9bdb181abfec6fb2d317468293e10b 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2019-02-23  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] Lazily create sentinel Map and Set buckets
>+        https://bugs.webkit.org/show_bug.cgi?id=194975
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        If VM::canUseJIT() returns false, we can lazily initialize sentinel Map and Set buckets.
>+        This patch adds getters to VM which lazily allocate these buckets. We eagerly initialize
>+        them if VM::canUseJIT() returns true since they can be touched from DFG and FTL.
>+
>+        * bytecode/BytecodeIntrinsicRegistry.cpp:
>+        (JSC::BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry):
>+        (JSC::BytecodeIntrinsicRegistry::sentinelMapBucketValue):
>+        (JSC::BytecodeIntrinsicRegistry::sentinelSetBucketValue):
>+        * bytecode/BytecodeIntrinsicRegistry.h:
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::handleIntrinsicCall):
>+        * dfg/DFGOperations.cpp:
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileGetMapBucketNext):
>+        * dfg/DFGSpeculativeJIT64.cpp:
>+        (JSC::DFG::SpeculativeJIT::compile):
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::compileGetMapBucket):
>+        (JSC::FTL::DFG::LowerDFGToB3::compileGetMapBucketNext):
>+        * runtime/MapConstructor.cpp:
>+        (JSC::mapPrivateFuncMapBucketNext):
>+        * runtime/SetConstructor.cpp:
>+        (JSC::setPrivateFuncSetBucketNext):
>+        * runtime/VM.cpp:
>+        (JSC::VM::VM):
>+        (JSC::VM::sentinelSetBucketSlow):
>+        (JSC::VM::sentinelMapBucketSlow):
>+        * runtime/VM.h:
>+        (JSC::VM::sentinelSetBucket):
>+        (JSC::VM::sentinelMapBucket):
>+
> 2019-02-22  Robin Morisset  <rmorisset@apple.com>
> 
>         DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit
>diff --git a/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp b/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp
>index 329ffe842cc81b986c667a2f084144d7c17cf016..d1ef00921116f034298a2b2332e762ee4ca10cac 100644
>--- a/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp
>+++ b/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.cpp
>@@ -69,8 +69,6 @@ BytecodeIntrinsicRegistry::BytecodeIntrinsicRegistry(VM& vm)
>     m_promiseStatePending.set(m_vm, jsNumber(static_cast<unsigned>(JSPromise::Status::Pending)));
>     m_promiseStateFulfilled.set(m_vm, jsNumber(static_cast<unsigned>(JSPromise::Status::Fulfilled)));
>     m_promiseStateRejected.set(m_vm, jsNumber(static_cast<unsigned>(JSPromise::Status::Rejected)));
>-    m_sentinelMapBucket.set(m_vm, m_vm.sentinelMapBucket.get());
>-    m_sentinelSetBucket.set(m_vm, m_vm.sentinelSetBucket.get());
>     m_GeneratorResumeModeNormal.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorResumeMode::NormalMode)));
>     m_GeneratorResumeModeThrow.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorResumeMode::ThrowMode)));
>     m_GeneratorResumeModeReturn.set(m_vm, jsNumber(static_cast<int32_t>(JSGeneratorFunction::GeneratorResumeMode::ReturnMode)));
>@@ -101,8 +99,18 @@ BytecodeIntrinsicNode::EmitterType BytecodeIntrinsicRegistry::lookup(const Ident
>     { \
>         return m_##name.get(); \
>     }
>-    JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_EACH_NAME(JSC_DECLARE_BYTECODE_INTRINSIC_CONSTANT_GENERATORS)
>+    JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_SIMPLE_EACH_NAME(JSC_DECLARE_BYTECODE_INTRINSIC_CONSTANT_GENERATORS)
> #undef JSC_DECLARE_BYTECODE_INTRINSIC_CONSTANT_GENERATORS
> 
>+JSValue BytecodeIntrinsicRegistry::sentinelMapBucketValue(BytecodeGenerator& generator)
>+{
>+    return generator.vm()->sentinelMapBucket();
>+}
>+
>+JSValue BytecodeIntrinsicRegistry::sentinelSetBucketValue(BytecodeGenerator& generator)
>+{
>+    return generator.vm()->sentinelSetBucket();
>+}
>+
> } // namespace JSC
> 
>diff --git a/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h b/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h
>index e97bd95b917f60ac21d9c82a70b58d54c1e874a8..42f8e1275c11790b2b0bed441885e5048ea24916 100644
>--- a/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h
>+++ b/Source/JavaScriptCore/bytecode/BytecodeIntrinsicRegistry.h
>@@ -66,6 +66,10 @@ class Identifier;
>     macro(defineEnumerableWritableConfigurableDataProperty) \
> 
> #define JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_EACH_NAME(macro) \
>+    JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_SIMPLE_EACH_NAME(macro) \
>+    JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_CUSTOM_EACH_NAME(macro) \
>+
>+#define JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_SIMPLE_EACH_NAME(macro) \
>     macro(undefined) \
>     macro(Infinity) \
>     macro(iterationKindKey) \
>@@ -85,8 +89,6 @@ class Identifier;
>     macro(promiseStatePending) \
>     macro(promiseStateFulfilled) \
>     macro(promiseStateRejected) \
>-    macro(sentinelMapBucket) \
>-    macro(sentinelSetBucket) \
>     macro(GeneratorResumeModeNormal) \
>     macro(GeneratorResumeModeThrow) \
>     macro(GeneratorResumeModeReturn) \
>@@ -101,6 +103,10 @@ class Identifier;
>     macro(AsyncGeneratorSuspendReasonAwait) \
>     macro(AsyncGeneratorSuspendReasonNone) \
> 
>+#define JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_CUSTOM_EACH_NAME(macro) \
>+    macro(sentinelMapBucket) \
>+    macro(sentinelSetBucket) \
>+
> class BytecodeIntrinsicRegistry {
>     WTF_MAKE_FAST_ALLOCATED;
>     WTF_MAKE_NONCOPYABLE(BytecodeIntrinsicRegistry);
>@@ -120,7 +126,7 @@ class BytecodeIntrinsicRegistry {
>     HashMap<RefPtr<UniquedStringImpl>, EmitterType, IdentifierRepHash> m_bytecodeIntrinsicMap;
> 
> #define JSC_DECLARE_BYTECODE_INTRINSIC_CONSTANT_GENERATORS(name) Strong<Unknown> m_##name;
>-    JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_EACH_NAME(JSC_DECLARE_BYTECODE_INTRINSIC_CONSTANT_GENERATORS)
>+    JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_SIMPLE_EACH_NAME(JSC_DECLARE_BYTECODE_INTRINSIC_CONSTANT_GENERATORS)
> #undef JSC_DECLARE_BYTECODE_INTRINSIC_CONSTANT_GENERATORS
> };
> 
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index 010cc39f8878b41594a7ac9e4ea6c0d00d06eb02..69bf14a43086d8c1ef0597d1e8b45ec45b7771e6 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -2929,9 +2929,9 @@ bool ByteCodeParser::handleIntrinsicCall(Node* callee, VirtualRegister result, I
>             Node* bucket = addToGraph(GetMapBucket, OpInfo(0), Edge(mapOrSet, useKind), Edge(normalizedKey), Edge(hash));
>             JSCell* sentinel = nullptr;
>             if (intrinsic == JSMapHasIntrinsic)
>-                sentinel = m_vm->sentinelMapBucket.get();
>+                sentinel = m_vm->sentinelMapBucket();
>             else
>-                sentinel = m_vm->sentinelSetBucket.get();
>+                sentinel = m_vm->sentinelSetBucket();
> 
>             FrozenValue* frozenPointer = m_graph.freeze(sentinel);
>             Node* invertedResult = addToGraph(CompareEqPtr, OpInfo(frozenPointer), bucket);
>diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
>index 6a578266eb4a128327e53cdaacb7d861577c49ef..029b0913130d5c7db3fdfe709919ec9ec2c93e84 100644
>--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
>@@ -2892,7 +2892,7 @@ JSCell* JIT_OPERATION operationJSMapFindBucket(ExecState* exec, JSCell* map, Enc
>     NativeCallFrameTracer tracer(&vm, exec);
>     JSMap::BucketType** bucket = jsCast<JSMap*>(map)->findBucket(exec, JSValue::decode(key), hash);
>     if (!bucket)
>-        return vm.sentinelMapBucket.get();
>+        return vm.sentinelMapBucket();
>     return *bucket;
> }
> 
>@@ -2902,7 +2902,7 @@ JSCell* JIT_OPERATION operationJSSetFindBucket(ExecState* exec, JSCell* map, Enc
>     NativeCallFrameTracer tracer(&vm, exec);
>     JSSet::BucketType** bucket = jsCast<JSSet*>(map)->findBucket(exec, JSValue::decode(key), hash);
>     if (!bucket)
>-        return vm.sentinelSetBucket.get();
>+        return vm.sentinelSetBucket();
>     return *bucket;
> }
> 
>@@ -2912,7 +2912,7 @@ JSCell* JIT_OPERATION operationSetAdd(ExecState* exec, JSCell* set, EncodedJSVal
>     NativeCallFrameTracer tracer(&vm, exec);
>     auto* bucket = jsCast<JSSet*>(set)->addNormalized(exec, JSValue::decode(key), JSValue(), hash);
>     if (!bucket)
>-        return vm.sentinelSetBucket.get();
>+        return vm.sentinelSetBucket();
>     return bucket;
> }
> 
>@@ -2922,7 +2922,7 @@ JSCell* JIT_OPERATION operationMapSet(ExecState* exec, JSCell* map, EncodedJSVal
>     NativeCallFrameTracer tracer(&vm, exec);
>     auto* bucket = jsCast<JSMap*>(map)->addNormalized(exec, JSValue::decode(key), JSValue::decode(value), hash);
>     if (!bucket)
>-        return vm.sentinelMapBucket.get();
>+        return vm.sentinelMapBucket();
>     return bucket;
> }
> 
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>index 2911288ede8d5699066d0d11ff2b7fd4b29de121..9e79a5a8d057405a577d7e7c5cb20c83446cd307 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>@@ -11792,10 +11792,10 @@ void SpeculativeJIT::compileGetMapBucketNext(Node* node)
>     notBucket.link(&m_jit);
>     JSCell* sentinel = nullptr;
>     if (node->bucketOwnerType() == BucketOwnerType::Map)
>-        sentinel = m_jit.vm()->sentinelMapBucket.get();
>+        sentinel = m_jit.vm()->sentinelMapBucket();
>     else {
>         ASSERT(node->bucketOwnerType() == BucketOwnerType::Set);
>-        sentinel = m_jit.vm()->sentinelSetBucket.get();
>+        sentinel = m_jit.vm()->sentinelSetBucket();
>     }
>     m_jit.move(TrustedImmPtr::weakPointer(m_jit.graph(), sentinel), resultGPR);
>     done.link(&m_jit);
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>index db8df894e336261cef5b6d33ead857c78f894b82..44b23ff213b22d6eeb747c95e51af34e3cc8fc20 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
>@@ -4217,9 +4217,9 @@ void SpeculativeJIT::compile(Node* node)
> 
>         notPresentInTable.link(&m_jit);
>         if (node->child1().useKind() == MapObjectUse)
>-            m_jit.move(TrustedImmPtr::weakPointer(m_jit.graph(), m_jit.vm()->sentinelMapBucket.get()), resultGPR);
>+            m_jit.move(TrustedImmPtr::weakPointer(m_jit.graph(), m_jit.vm()->sentinelMapBucket()), resultGPR);
>         else
>-            m_jit.move(TrustedImmPtr::weakPointer(m_jit.graph(), m_jit.vm()->sentinelSetBucket.get()), resultGPR);
>+            m_jit.move(TrustedImmPtr::weakPointer(m_jit.graph(), m_jit.vm()->sentinelSetBucket()), resultGPR);
>         done.link(&m_jit);
>         cellResult(resultGPR, node);
>         break;
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index 87de9baba05c1cedc843fd751240a805a902734e..f8a79121894cd30147e0fb8b89e91420b21263f3 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -9744,9 +9744,9 @@ class LowerDFGToB3 {
>         m_out.appendTo(notPresentInTable, continuation);
>         ValueFromBlock notPresentResult;
>         if (m_node->child1().useKind() == MapObjectUse)
>-            notPresentResult = m_out.anchor(weakPointer(vm().sentinelMapBucket.get()));
>+            notPresentResult = m_out.anchor(weakPointer(vm().sentinelMapBucket()));
>         else if (m_node->child1().useKind() == SetObjectUse)
>-            notPresentResult = m_out.anchor(weakPointer(vm().sentinelSetBucket.get()));
>+            notPresentResult = m_out.anchor(weakPointer(vm().sentinelSetBucket()));
>         else
>             RELEASE_ASSERT_NOT_REACHED();
>         m_out.jump(continuation);
>@@ -9792,10 +9792,10 @@ class LowerDFGToB3 {
>         m_out.appendTo(noBucket, hasBucket);
>         ValueFromBlock noBucketResult;
>         if (m_node->bucketOwnerType() == BucketOwnerType::Map)
>-            noBucketResult = m_out.anchor(weakPointer(vm().sentinelMapBucket.get()));
>+            noBucketResult = m_out.anchor(weakPointer(vm().sentinelMapBucket()));
>         else {
>             ASSERT(m_node->bucketOwnerType() == BucketOwnerType::Set);
>-            noBucketResult = m_out.anchor(weakPointer(vm().sentinelSetBucket.get()));
>+            noBucketResult = m_out.anchor(weakPointer(vm().sentinelSetBucket()));
>         }
>         m_out.jump(continuation);
> 
>diff --git a/Source/JavaScriptCore/runtime/MapConstructor.cpp b/Source/JavaScriptCore/runtime/MapConstructor.cpp
>index 78e534bc06c914e7bd851010e74f497b8470bcef..5ee2cbf258662c6d31d56219f9c0cb2ea2fbf56d 100644
>--- a/Source/JavaScriptCore/runtime/MapConstructor.cpp
>+++ b/Source/JavaScriptCore/runtime/MapConstructor.cpp
>@@ -136,7 +136,7 @@ EncodedJSValue JSC_HOST_CALL mapPrivateFuncMapBucketNext(ExecState* exec)
>             return JSValue::encode(bucket);
>         bucket = bucket->next();
>     }
>-    return JSValue::encode(exec->vm().sentinelMapBucket.get());
>+    return JSValue::encode(exec->vm().sentinelMapBucket());
> }
> 
> EncodedJSValue JSC_HOST_CALL mapPrivateFuncMapBucketKey(ExecState* exec)
>diff --git a/Source/JavaScriptCore/runtime/SetConstructor.cpp b/Source/JavaScriptCore/runtime/SetConstructor.cpp
>index 00b7f93294e29ce295d8f136af9071881e08ba8d..f60bfce600d9eb0240a2aa14afe271cf843b6749 100644
>--- a/Source/JavaScriptCore/runtime/SetConstructor.cpp
>+++ b/Source/JavaScriptCore/runtime/SetConstructor.cpp
>@@ -122,7 +122,7 @@ EncodedJSValue JSC_HOST_CALL setPrivateFuncSetBucketNext(ExecState* exec)
>             return JSValue::encode(bucket);
>         bucket = bucket->next();
>     }
>-    return JSValue::encode(exec->vm().sentinelSetBucket.get());
>+    return JSValue::encode(exec->vm().sentinelSetBucket());
> }
> 
> EncodedJSValue JSC_HOST_CALL setPrivateFuncSetBucketKey(ExecState* exec)
>diff --git a/Source/JavaScriptCore/runtime/VM.cpp b/Source/JavaScriptCore/runtime/VM.cpp
>index 0d1efc94c4013bd2f3b3bb9715aa171f6de8f9c8..15a72deebdae4fc9da62632f9b05dc8cc1159446 100644
>--- a/Source/JavaScriptCore/runtime/VM.cpp
>+++ b/Source/JavaScriptCore/runtime/VM.cpp
>@@ -401,8 +401,11 @@ VM::VM(VMType vmType, HeapType heapType)
>     bigIntStructure.set(*this, JSBigInt::createStructure(*this, 0, jsNull()));
>     executableToCodeBlockEdgeStructure.set(*this, ExecutableToCodeBlockEdge::createStructure(*this, nullptr, jsNull()));
> 
>-    sentinelSetBucket.set(*this, JSSet::BucketType::createSentinel(*this));
>-    sentinelMapBucket.set(*this, JSMap::BucketType::createSentinel(*this));
>+    // Eagerly initialize constant cells if concurrent compiler can touch.
>+    if (canUseJIT()) {
>+        sentinelMapBucket();
>+        sentinelSetBucket();
>+    }
> 
>     Thread::current().setCurrentAtomicStringTable(existingEntryAtomicStringTable);
> 
>@@ -1284,6 +1287,23 @@ DYNAMIC_SPACE_AND_SET_DEFINE_MEMBER_SLOW(moduleProgramExecutableSpace, destructi
> 
> #undef DYNAMIC_SPACE_AND_SET_DEFINE_MEMBER_SLOW
> 
>+
>+JSCell* VM::sentinelSetBucketSlow()
>+{
>+    ASSERT(!m_sentinelSetBucket);
>+    auto* sentinel = JSSet::BucketType::createSentinel(*this);
>+    m_sentinelSetBucket.set(*this, sentinel);
>+    return sentinel;
>+}
>+
>+JSCell* VM::sentinelMapBucketSlow()
>+{
>+    ASSERT(!m_sentinelMapBucket);
>+    auto* sentinel = JSMap::BucketType::createSentinel(*this);
>+    m_sentinelMapBucket.set(*this, sentinel);
>+    return sentinel;
>+}
>+
> JSGlobalObject* VM::vmEntryGlobalObject(const CallFrame* callFrame) const
> {
>     if (callFrame && callFrame->isGlobalExec()) {
>diff --git a/Source/JavaScriptCore/runtime/VM.h b/Source/JavaScriptCore/runtime/VM.h
>index 5ab352ff61c572f87436614ffd20555b33976e0c..84cccdaddd9548c7d6b14fbd507cba6b6de480ed 100644
>--- a/Source/JavaScriptCore/runtime/VM.h
>+++ b/Source/JavaScriptCore/runtime/VM.h
>@@ -539,8 +539,9 @@ class VM : public ThreadSafeRefCounted<VM>, public DoublyLinkedListNode<VM> {
>     Strong<Structure> executableToCodeBlockEdgeStructure;
> 
>     Strong<JSCell> emptyPropertyNameEnumerator;
>-    Strong<JSCell> sentinelSetBucket;
>-    Strong<JSCell> sentinelMapBucket;
>+
>+    Strong<JSCell> m_sentinelSetBucket;
>+    Strong<JSCell> m_sentinelMapBucket;
> 
>     std::unique_ptr<PromiseDeferredTimer> promiseDeferredTimer;
>     
>@@ -562,6 +563,22 @@ class VM : public ThreadSafeRefCounted<VM>, public DoublyLinkedListNode<VM> {
>     AtomicStringTable* atomicStringTable() const { return m_atomicStringTable; }
>     WTF::SymbolRegistry& symbolRegistry() { return m_symbolRegistry; }
> 
>+    JSCell* sentinelSetBucket()
>+    {
>+        if (LIKELY(m_sentinelSetBucket))
>+            return m_sentinelSetBucket.get();
>+        return sentinelSetBucketSlow();
>+    }
>+    JSCell* sentinelSetBucketSlow();
>+
>+    JSCell* sentinelMapBucket()
>+    {
>+        if (LIKELY(m_sentinelMapBucket))
>+            return m_sentinelMapBucket.get();
>+        return sentinelMapBucketSlow();
>+    }
>+    JSCell* sentinelMapBucketSlow();
>+
>     WeakGCMap<SymbolImpl*, Symbol, PtrHash<SymbolImpl*>> symbolImplToSymbolMap;
> 
>     enum class DeletePropertyMode {

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194975: 362819 | 362820