[webkit-changes] [245667] trunk

Wed, 22 May 2019 18:23:16 -0700

Title: [245667] trunk
Revision
245667
Author
[email protected]
Date
2019-05-22 18:22:33 -0700 (Wed, 22 May 2019)

Log Message

[JSC] ArrayAllocationProfile should not access to butterfly in concurrent compiler
https://bugs.webkit.org/show_bug.cgi?id=197809

Reviewed by Michael Saboff.

JSTests:

* stress/array-allocation-profile-should-not-update-itself-in-concurrent-compiler.js: Added.
(foo):

Source/_javascript_Core:

ArrayAllocationProfile assumes that Butterfly can be accessed concurrently. But this is not correct now
since LargeAllocation Butterfly can be realloced. In this patch, we switch profiling array allocations
only in the main thread. This allocation profiling is repeatedly called in the main thread's slow path,
and it is also called when updating the profiles in the main thread.

We also rename updateAllPredictionsAndCountLiveness to updateAllValueProfilePredictionsAndCountLiveness
since it only cares ValueProfiles.

* bytecode/ArrayAllocationProfile.cpp:
(JSC::ArrayAllocationProfile::updateProfile):
* bytecode/ArrayAllocationProfile.h:
(JSC::ArrayAllocationProfile::selectIndexingTypeConcurrently):
(JSC::ArrayAllocationProfile::selectIndexingType):
(JSC::ArrayAllocationProfile::vectorLengthHintConcurrently):
(JSC::ArrayAllocationProfile::vectorLengthHint):
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::updateAllValueProfilePredictionsAndCountLiveness):
(JSC::CodeBlock::updateAllValueProfilePredictions):
(JSC::CodeBlock::shouldOptimizeNow):
(JSC::CodeBlock::updateAllPredictionsAndCountLiveness): Deleted.
* bytecode/CodeBlock.h:
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (245666 => 245667)


--- trunk/JSTests/ChangeLog	2019-05-23 00:56:49 UTC (rev 245666)
+++ trunk/JSTests/ChangeLog	2019-05-23 01:22:33 UTC (rev 245667)
@@ -1,3 +1,13 @@
+2019-05-22  Yusuke Suzuki  <[email protected]>
+
+        [JSC] ArrayAllocationProfile should not access to butterfly in concurrent compiler
+        https://bugs.webkit.org/show_bug.cgi?id=197809
+
+        Reviewed by Michael Saboff.
+
+        * stress/array-allocation-profile-should-not-update-itself-in-concurrent-compiler.js: Added.
+        (foo):
+
 2019-05-22  Ross Kirsling  <[email protected]>
 
         [ESNext] Implement support for Numeric Separators

Added: trunk/JSTests/stress/array-allocation-profile-should-not-update-itself-in-concurrent-compiler.js (0 => 245667)


--- trunk/JSTests/stress/array-allocation-profile-should-not-update-itself-in-concurrent-compiler.js	                        (rev 0)
+++ trunk/JSTests/stress/array-allocation-profile-should-not-update-itself-in-concurrent-compiler.js	2019-05-23 01:22:33 UTC (rev 245667)
@@ -0,0 +1,11 @@
+//@ runDefault("--jitPolicyScale=0", "--useArrayAllocationProfiling=0")
+function foo() {
+    for (let i = 0; i < 30; i++) {
+        const ar = [];
+        for (let j = 0; j <= 1500; j++) {
+            ar[j] = null;
+        }
+    }
+}
+
+foo();

Modified: trunk/Source/_javascript_Core/ChangeLog (245666 => 245667)


--- trunk/Source/_javascript_Core/ChangeLog	2019-05-23 00:56:49 UTC (rev 245666)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-05-23 01:22:33 UTC (rev 245667)
@@ -1,5 +1,36 @@
 2019-05-22  Yusuke Suzuki  <[email protected]>
 
+        [JSC] ArrayAllocationProfile should not access to butterfly in concurrent compiler
+        https://bugs.webkit.org/show_bug.cgi?id=197809
+
+        Reviewed by Michael Saboff.
+
+        ArrayAllocationProfile assumes that Butterfly can be accessed concurrently. But this is not correct now
+        since LargeAllocation Butterfly can be realloced. In this patch, we switch profiling array allocations
+        only in the main thread. This allocation profiling is repeatedly called in the main thread's slow path,
+        and it is also called when updating the profiles in the main thread.
+
+        We also rename updateAllPredictionsAndCountLiveness to updateAllValueProfilePredictionsAndCountLiveness
+        since it only cares ValueProfiles.
+
+        * bytecode/ArrayAllocationProfile.cpp:
+        (JSC::ArrayAllocationProfile::updateProfile):
+        * bytecode/ArrayAllocationProfile.h:
+        (JSC::ArrayAllocationProfile::selectIndexingTypeConcurrently):
+        (JSC::ArrayAllocationProfile::selectIndexingType):
+        (JSC::ArrayAllocationProfile::vectorLengthHintConcurrently):
+        (JSC::ArrayAllocationProfile::vectorLengthHint):
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::updateAllValueProfilePredictionsAndCountLiveness):
+        (JSC::CodeBlock::updateAllValueProfilePredictions):
+        (JSC::CodeBlock::shouldOptimizeNow):
+        (JSC::CodeBlock::updateAllPredictionsAndCountLiveness): Deleted.
+        * bytecode/CodeBlock.h:
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+
+2019-05-22  Yusuke Suzuki  <[email protected]>
+
         [JSC] Shrink Metadata
         https://bugs.webkit.org/show_bug.cgi?id=197940

Modified: trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp (245666 => 245667)


--- trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp	2019-05-23 00:56:49 UTC (rev 245666)
+++ trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.cpp	2019-05-23 01:22:33 UTC (rev 245667)
@@ -47,7 +47,13 @@
     //   it's possible for that array to no longer be reachable, it cannot actually
     //   be freed, since we require the GC to wait until all concurrent JITing
     //   finishes.
+    //
+    // But one exception is vector length. We access vector length to get the vector
+    // length hint. However vector length can be accessible only from the main
+    // thread because large butterfly can be realloced in the main thread.
+    // So for now, we update the allocation profile only from the main thread.
     
+    ASSERT(!isCompilationThread());
     JSArray* lastArray = m_lastArray;
     if (!lastArray)
         return;

Modified: trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.h (245666 => 245667)


--- trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.h	2019-05-23 00:56:49 UTC (rev 245666)
+++ trunk/Source/_javascript_Core/bytecode/ArrayAllocationProfile.h	2019-05-23 01:22:33 UTC (rev 245667)
@@ -39,8 +39,14 @@
         initializeIndexingMode(recommendedIndexingMode);
     }
 
+    IndexingType selectIndexingTypeConcurrently()
+    {
+        return m_currentIndexingType;
+    }
+
     IndexingType selectIndexingType()
     {
+        ASSERT(!isCompilationThread());
         JSArray* lastArray = m_lastArray;
         if (lastArray && UNLIKELY(lastArray->indexingType() != m_currentIndexingType))
             updateProfile();
@@ -48,8 +54,14 @@
     }
 
     // vector length hint becomes [0, BASE_CONTIGUOUS_VECTOR_LEN_MAX].
+    unsigned vectorLengthHintConcurrently()
+    {
+        return m_largestSeenVectorLength;
+    }
+
     unsigned vectorLengthHint()
     {
+        ASSERT(!isCompilationThread());
         JSArray* lastArray = m_lastArray;
         if (lastArray && (m_largestSeenVectorLength != BASE_CONTIGUOUS_VECTOR_LEN_MAX) && UNLIKELY(lastArray->getVectorLength() > m_largestSeenVectorLength))
             updateProfile();

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (245666 => 245667)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2019-05-23 00:56:49 UTC (rev 245666)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp	2019-05-23 01:22:33 UTC (rev 245667)
@@ -2626,7 +2626,7 @@
 }
 #endif // ENABLE(DFG_JIT)
 
-void CodeBlock::updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles)
+void CodeBlock::updateAllValueProfilePredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles)
 {
     ConcurrentJSLocker locker(m_lock);
 
@@ -2664,7 +2664,7 @@
 void CodeBlock::updateAllValueProfilePredictions()
 {
     unsigned ignoredValue1, ignoredValue2;
-    updateAllPredictionsAndCountLiveness(ignoredValue1, ignoredValue2);
+    updateAllValueProfilePredictionsAndCountLiveness(ignoredValue1, ignoredValue2);
 }
 
 void CodeBlock::updateAllArrayPredictions()
@@ -2698,7 +2698,7 @@
     
     unsigned numberOfLiveNonArgumentValueProfiles;
     unsigned numberOfSamplesInProfiles;
-    updateAllPredictionsAndCountLiveness(numberOfLiveNonArgumentValueProfiles, numberOfSamplesInProfiles);
+    updateAllValueProfilePredictionsAndCountLiveness(numberOfLiveNonArgumentValueProfiles, numberOfSamplesInProfiles);
 
     if (Options::verboseOSR()) {
         dataLogF(

Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (245666 => 245667)


--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2019-05-23 00:56:49 UTC (rev 245666)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h	2019-05-23 01:22:33 UTC (rev 245667)
@@ -906,7 +906,7 @@
     
     double optimizationThresholdScalingFactor();
 
-    void updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles);
+    void updateAllValueProfilePredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles);
 
     void setConstantIdentifierSetRegisters(VM&, const Vector<ConstantIdentifierSetEntry>& constants);

Modified: trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp (245666 => 245667)


--- trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2019-05-23 00:56:49 UTC (rev 245666)
+++ trunk/Source/_javascript_Core/dfg/DFGByteCodeParser.cpp	2019-05-23 01:22:33 UTC (rev 245667)
@@ -4885,8 +4885,8 @@
             ArrayAllocationProfile& profile = ""
             for (int operandIdx = startOperand; operandIdx > startOperand - numOperands; --operandIdx)
                 addVarArgChild(get(VirtualRegister(operandIdx)));
-            unsigned vectorLengthHint = std::max<unsigned>(profile.vectorLengthHint(), numOperands);
-            set(bytecode.m_dst, addToGraph(Node::VarArg, NewArray, OpInfo(profile.selectIndexingType()), OpInfo(vectorLengthHint)));
+            unsigned vectorLengthHint = std::max<unsigned>(profile.vectorLengthHintConcurrently(), numOperands);
+            set(bytecode.m_dst, addToGraph(Node::VarArg, NewArray, OpInfo(profile.selectIndexingTypeConcurrently()), OpInfo(vectorLengthHint)));
             NEXT_OPCODE(op_new_array);
         }
 
@@ -4916,7 +4916,7 @@
         case op_new_array_with_size: {
             auto bytecode = currentInstruction->as<OpNewArrayWithSize>();
             ArrayAllocationProfile& profile = ""
-            set(bytecode.m_dst, addToGraph(NewArrayWithSize, OpInfo(profile.selectIndexingType()), get(bytecode.m_length)));
+            set(bytecode.m_dst, addToGraph(NewArrayWithSize, OpInfo(profile.selectIndexingTypeConcurrently()), get(bytecode.m_length)));
             NEXT_OPCODE(op_new_array_with_size);
         }
             

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to