Attachment 369632 Details for Bug 197809 – Patch

[patch] Patch

bug-197809-20190510184006.patch (text/plain), 7.34 KB, created by Yusuke Suzuki on 2019-05-10 18:40:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-05-10 18:40:07 PDT

Size: 7.34 KB

patch

obsolete

>Subversion Revision: 245195
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 66f54129290b2cfa2daf9f0af3703158e975639e..04b2a53cc2ad49d8760d0d44a41d9203e9505f23 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-05-10  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] ArrayAllocationProfile should not access to butterfly in concurrent compiler
>+        https://bugs.webkit.org/show_bug.cgi?id=197809
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * bytecode/ArrayAllocationProfile.cpp:
>+        (JSC::ArrayAllocationProfile::updateProfile):
>+        * bytecode/ArrayAllocationProfile.h:
>+        (JSC::ArrayAllocationProfile::vectorLengthHint):
>+        * bytecode/CodeBlock.cpp:
>+        (JSC::CodeBlock::updateAllValueProfilePredictionsAndCountLiveness):
>+        (JSC::CodeBlock::updateAllValueProfilePredictions):
>+        (JSC::CodeBlock::shouldOptimizeNow):
>+        (JSC::CodeBlock::updateAllPredictionsAndCountLiveness): Deleted.
>+        * bytecode/CodeBlock.h:
>+
> 2019-05-10  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] String substring operation should return ropes consistently
>diff --git a/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp b/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp
>index bd73651ab22cdc1015b67fa8b8c366aeea133b2b..4335f4e529c788bed26de1e30bab1e4dc39e6aa7 100644
>--- a/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp
>+++ b/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.cpp
>@@ -47,6 +47,12 @@ void ArrayAllocationProfile::updateProfile()
>     //   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.
>+    //
>+    // 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. We only
>+    // update vector length hint from the main thread and it is OK because every time
>+    // compiler runs we anyway update array allocation profile.
>     
>     JSArray* lastArray = m_lastArray;
>     if (!lastArray)
>@@ -60,7 +66,8 @@ void ArrayAllocationProfile::updateProfile()
>             indexingType |= CopyOnWrite;
>         }
>         m_currentIndexingType = indexingType;
>-        m_largestSeenVectorLength = std::min(std::max(m_largestSeenVectorLength, lastArray->getVectorLength()), BASE_CONTIGUOUS_VECTOR_LEN_MAX);
>+        if (!isCompilationThread())
>+            m_largestSeenVectorLength = std::min(std::max(m_largestSeenVectorLength, lastArray->getVectorLength()), BASE_CONTIGUOUS_VECTOR_LEN_MAX);
>     }
>     m_lastArray = nullptr;
> }
>diff --git a/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.h b/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.h
>index fef936e257888ed22ea20f8b18f9d9c7b13265f3..d1556229d0e35dc1b9f5c6a5c669d3eacac19b28 100644
>--- a/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.h
>+++ b/Source/JavaScriptCore/bytecode/ArrayAllocationProfile.h
>@@ -51,8 +51,10 @@ class ArrayAllocationProfile {
>     unsigned vectorLengthHint()
>     {
>         JSArray* lastArray = m_lastArray;
>-        if (lastArray && (m_largestSeenVectorLength != BASE_CONTIGUOUS_VECTOR_LEN_MAX) && UNLIKELY(lastArray->getVectorLength() > m_largestSeenVectorLength))
>-            updateProfile();
>+        if (lastArray && (m_largestSeenVectorLength != BASE_CONTIGUOUS_VECTOR_LEN_MAX) && !isCompilationThread()) {
>+            if (UNLIKELY(lastArray->getVectorLength() > m_largestSeenVectorLength))
>+                updateProfile();
>+        }
>         return m_largestSeenVectorLength;
>     }
>     
>diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>index db28f67e2d17b30395500484e86945a7ce1e9d8a..5d1d61d127c045282e4b9b36553ea49cd5c8431b 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>@@ -2626,7 +2626,7 @@ const Identifier& CodeBlock::identifier(int index) const
> }
> #endif // ENABLE(DFG_JIT)
> 
>-void CodeBlock::updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles)
>+void CodeBlock::updateAllValueProfilePredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles)
> {
>     ConcurrentJSLocker locker(m_lock);
> 
>@@ -2663,7 +2663,7 @@ void CodeBlock::updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonAr
> void CodeBlock::updateAllValueProfilePredictions()
> {
>     unsigned ignoredValue1, ignoredValue2;
>-    updateAllPredictionsAndCountLiveness(ignoredValue1, ignoredValue2);
>+    updateAllValueProfilePredictionsAndCountLiveness(ignoredValue1, ignoredValue2);
> }
> 
> void CodeBlock::updateAllArrayPredictions()
>@@ -2697,7 +2697,7 @@ bool CodeBlock::shouldOptimizeNow()
>     
>     unsigned numberOfLiveNonArgumentValueProfiles;
>     unsigned numberOfSamplesInProfiles;
>-    updateAllPredictionsAndCountLiveness(numberOfLiveNonArgumentValueProfiles, numberOfSamplesInProfiles);
>+    updateAllValueProfilePredictionsAndCountLiveness(numberOfLiveNonArgumentValueProfiles, numberOfSamplesInProfiles);
> 
>     if (Options::verboseOSR()) {
>         dataLogF(
>diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h
>index a4c23bf8688111242918ba372deeccae739e8889..fc776933ad463295188584bd27c47def96573078 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.h
>@@ -907,7 +907,7 @@ class CodeBlock : public JSCell {
>     
>     double optimizationThresholdScalingFactor();
> 
>-    void updateAllPredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles);
>+    void updateAllValueProfilePredictionsAndCountLiveness(unsigned& numberOfLiveNonArgumentValueProfiles, unsigned& numberOfSamplesInProfiles);
> 
>     void setConstantIdentifierSetRegisters(VM&, const Vector<ConstantIdentifierSetEntry>& constants);
> 
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 424710cd97c89eb5bf67c1dc7969227468f8e2a4..ed5870c8e977dfa508a19031f93a2e1a55529844 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-05-10  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] ArrayAllocationProfile should not access to butterfly in concurrent compiler
>+        https://bugs.webkit.org/show_bug.cgi?id=197809
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/concurrent-compiler-should-not-touch-butterfly-if-it-is-not-array-storage.js: Added.
>+        (foo):
>+
> 2019-05-10  Keith Miller  <keith_miller@apple.com>
> 
>         Update test262 tests.
>diff --git a/JSTests/stress/concurrent-compiler-should-not-touch-butterfly-if-it-is-not-array-storage.js b/JSTests/stress/concurrent-compiler-should-not-touch-butterfly-if-it-is-not-array-storage.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..ce5c6bfb09b6ed0e0f092dd197ea3e21e4bbfec3
>--- /dev/null
>+++ b/JSTests/stress/concurrent-compiler-should-not-touch-butterfly-if-it-is-not-array-storage.js
>@@ -0,0 +1,12 @@
>+//@ 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();

Actions: View | Formatted Diff | Diff

Attachments on bug 197809: 369632 | 370456 | 370457