Title: [181456] trunk/Source/_javascript_Core
- Revision
- 181456
- Author
- [email protected]
- Date
- 2015-03-12 15:14:32 -0700 (Thu, 12 Mar 2015)
Log Message
Use std::atomic for CodeBlock::m_visitAggregateHasBeenCalled.
<https://webkit.org/b/142640>
Reviewed by Mark Hahnenberg.
We used to spin our own compare and swap on a uint8_t. Now that we can
use C++11, let's use std::atomic instead.
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::visitAggregate):
- The CAS here needs std::memory_order_acquire ordering because it
requires lock acquisition semantics to visit the CodeBlock.
* bytecode/CodeBlock.h:
(JSC::CodeBlockSet::mark):
* heap/CodeBlockSet.cpp:
(JSC::CodeBlockSet::clearMarksForFullCollection):
(JSC::CodeBlockSet::clearMarksForEdenCollection):
- These can go with relaxed ordering because they are all done before
the GC starts parallel marking.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (181455 => 181456)
--- trunk/Source/_javascript_Core/ChangeLog 2015-03-12 21:01:37 UTC (rev 181455)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-03-12 22:14:32 UTC (rev 181456)
@@ -1,3 +1,26 @@
+2015-03-12 Mark Lam <[email protected]>
+
+ Use std::atomic for CodeBlock::m_visitAggregateHasBeenCalled.
+ <https://webkit.org/b/142640>
+
+ Reviewed by Mark Hahnenberg.
+
+ We used to spin our own compare and swap on a uint8_t. Now that we can
+ use C++11, let's use std::atomic instead.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::visitAggregate):
+ - The CAS here needs std::memory_order_acquire ordering because it
+ requires lock acquisition semantics to visit the CodeBlock.
+
+ * bytecode/CodeBlock.h:
+ (JSC::CodeBlockSet::mark):
+ * heap/CodeBlockSet.cpp:
+ (JSC::CodeBlockSet::clearMarksForFullCollection):
+ (JSC::CodeBlockSet::clearMarksForEdenCollection):
+ - These can go with relaxed ordering because they are all done before
+ the GC starts parallel marking.
+
2015-03-12 Csaba Osztrogonác <[email protected]>
[cmake] Fix the incremental build issue revealed by r181419
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (181455 => 181456)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2015-03-12 21:01:37 UTC (rev 181455)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2015-03-12 22:14:32 UTC (rev 181456)
@@ -2195,26 +2195,12 @@
{
#if ENABLE(PARALLEL_GC)
// I may be asked to scan myself more than once, and it may even happen concurrently.
- // To this end, use a CAS loop to check if I've been called already. Only one thread
- // may proceed past this point - whichever one wins the CAS race.
- unsigned oldValue;
- do {
- oldValue = m_visitAggregateHasBeenCalled;
- if (oldValue) {
- // Looks like someone else won! Return immediately to ensure that we don't
- // trace the same CodeBlock concurrently. Doing so is hazardous since we will
- // be mutating the state of ValueProfiles, which contain JSValues, which can
- // have word-tearing on 32-bit, leading to awesome timing-dependent crashes
- // that are nearly impossible to track down.
-
- // Also note that it must be safe to return early as soon as we see the
- // value true (well, (unsigned)1), since once a GC thread is in this method
- // and has won the CAS race (i.e. was responsible for setting the value true)
- // it will definitely complete the rest of this method before declaring
- // termination.
- return;
- }
- } while (!WTF::weakCompareAndSwap(&m_visitAggregateHasBeenCalled, 0, 1));
+ // To this end, use an atomic operation to check (and set) if I've been called already.
+ // Only one thread may proceed past this point - whichever one wins the atomic set race.
+ bool expected = false;
+ bool setByMe = m_visitAggregateHasBeenCalled.compare_exchange_strong(expected, true, std::memory_order_acquire);
+ if (!setByMe)
+ return;
#endif // ENABLE(PARALLEL_GC)
if (!!m_alternative)
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (181455 => 181456)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2015-03-12 21:01:37 UTC (rev 181455)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2015-03-12 22:14:32 UTC (rev 181456)
@@ -1065,7 +1065,7 @@
bool m_isStrictMode;
bool m_needsActivation;
bool m_mayBeExecuting;
- uint8_t m_visitAggregateHasBeenCalled;
+ std::atomic<bool> m_visitAggregateHasBeenCalled;
RefPtr<SourceProvider> m_source;
unsigned m_sourceOffset;
@@ -1301,7 +1301,7 @@
codeBlock->m_mayBeExecuting = true;
// We might not have cleared the marks for this CodeBlock, but we need to visit it.
- codeBlock->m_visitAggregateHasBeenCalled = false;
+ codeBlock->m_visitAggregateHasBeenCalled.store(false, std::memory_order_relaxed);
#if ENABLE(GGC)
m_currentlyExecuting.append(codeBlock);
#endif
Modified: trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp (181455 => 181456)
--- trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp 2015-03-12 21:01:37 UTC (rev 181455)
+++ trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp 2015-03-12 22:14:32 UTC (rev 181456)
@@ -65,7 +65,7 @@
{
for (CodeBlock* codeBlock : m_oldCodeBlocks) {
codeBlock->m_mayBeExecuting = false;
- codeBlock->m_visitAggregateHasBeenCalled = false;
+ codeBlock->m_visitAggregateHasBeenCalled.store(false, std::memory_order_relaxed);
}
// We promote after we clear marks on the old generation CodeBlocks because
@@ -82,7 +82,7 @@
continue;
executable->forEachCodeBlock([](CodeBlock* codeBlock) {
codeBlock->m_mayBeExecuting = false;
- codeBlock->m_visitAggregateHasBeenCalled = false;
+ codeBlock->m_visitAggregateHasBeenCalled.store(false, std::memory_order_relaxed);
});
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
