Diff
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/ChangeLog (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/ChangeLog 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/ChangeLog 2016-10-11 07:38:03 UTC (rev 207061)
@@ -1,3 +1,62 @@
+2016-09-08 Filip Pizlo <fpi...@apple.com>
+
+ Heap::isMarked() shouldn't pay the price of concurrent lazy flipping
+ https://bugs.webkit.org/show_bug.cgi?id=161760
+
+ Reviewed by Mark Lam.
+
+ To fix a race condition in marking, I made Heap::isMarked() and Heap::isLive() atomic by
+ using flipIfNecessaryConcurrently() instead of flipIfNecessary().
+
+ This introduces three unnecessary overheads:
+
+ - isLive() is not called by marking, so that change was not necessary.
+
+ - isMarked() gets calls many times outside of marking, so it shouldn't always do the
+ concurrent thing. This adds isMarkedConcurrently() for use in marking, and reverts
+ isMarked().
+
+ - isMarked() and isMarkedConcurrently() don't actually have to do the lazy flip. They can
+ return false if the flip is necessary.
+
+ I added a bunch of debug assertions to make sure that isLive() and isMarked() are not called
+ during marking.
+
+ If we needed to, we could remove most of the calls to isMarkedConcurrently(). As a kind of
+ optimization, CodeBlock does an initial fixpoint iteration during marking, and so all of the
+ code called from CodeBlock's fixpoint iterator needs to use isMarkedConcurrently(). But we
+ could probably arrange for CodeBlock only do fixpoint iterating during the weak reference
+ thing.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::visitWeakly):
+ (JSC::CodeBlock::shouldJettisonDueToOldAge):
+ (JSC::shouldMarkTransition):
+ (JSC::CodeBlock::propagateTransitions):
+ (JSC::CodeBlock::determineLiveness):
+ * bytecode/PolymorphicAccess.cpp:
+ (JSC::AccessCase::propagateTransitions):
+ * heap/Heap.h:
+ * heap/HeapInlines.h:
+ (JSC::Heap::isLive):
+ (JSC::Heap::isMarked):
+ (JSC::Heap::isMarkedConcurrently):
+ * heap/MarkedBlock.cpp:
+ (JSC::MarkedBlock::flipIfNecessarySlow):
+ (JSC::MarkedBlock::flipIfNecessaryConcurrentlySlow):
+ (JSC::MarkedBlock::needsFlip):
+ * heap/MarkedBlock.h:
+ (JSC::MarkedBlock::needsFlip):
+ (JSC::MarkedBlock::flipIfNecessary):
+ (JSC::MarkedBlock::flipIfNecessaryConcurrently):
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::appendToMarkStack):
+ (JSC::SlotVisitor::markAuxiliary):
+ (JSC::SlotVisitor::visitChildren):
+ * runtime/Structure.cpp:
+ (JSC::Structure::isCheapDuringGC):
+ (JSC::Structure::markIfCheap):
+
2016-09-08 Saam Barati <sbar...@apple.com>
We should inline operationConvertJSValueToBoolean into JIT code
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/bytecode/CodeBlock.cpp (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/bytecode/CodeBlock.cpp 2016-10-11 07:38:03 UTC (rev 207061)
@@ -2485,7 +2485,7 @@
if (!setByMe)
return;
- if (Heap::isMarked(this))
+ if (Heap::isMarkedConcurrently(this))
return;
if (shouldVisitStrongly()) {
@@ -2628,7 +2628,7 @@
bool CodeBlock::shouldJettisonDueToOldAge()
{
- if (Heap::isMarked(this))
+ if (Heap::isMarkedConcurrently(this))
return false;
if (UNLIKELY(Options::forceCodeBlockToJettisonDueToOldAge()))
@@ -2643,10 +2643,10 @@
#if ENABLE(DFG_JIT)
static bool shouldMarkTransition(DFG::WeakReferenceTransition& transition)
{
- if (transition.m_codeOrigin && !Heap::isMarked(transition.m_codeOrigin.get()))
+ if (transition.m_codeOrigin && !Heap::isMarkedConcurrently(transition.m_codeOrigin.get()))
return false;
- if (!Heap::isMarked(transition.m_from.get()))
+ if (!Heap::isMarkedConcurrently(transition.m_from.get()))
return false;
return true;
@@ -2677,7 +2677,7 @@
m_vm->heap.structureIDTable().get(oldStructureID);
Structure* newStructure =
m_vm->heap.structureIDTable().get(newStructureID);
- if (Heap::isMarked(oldStructure))
+ if (Heap::isMarkedConcurrently(oldStructure))
visitor.appendUnbarrieredReadOnlyPointer(newStructure);
else
allAreMarkedSoFar = false;
@@ -2749,7 +2749,7 @@
// GC we still have not proved liveness, then this code block is toast.
bool allAreLiveSoFar = true;
for (unsigned i = 0; i < dfgCommon->weakReferences.size(); ++i) {
- if (!Heap::isMarked(dfgCommon->weakReferences[i].get())) {
+ if (!Heap::isMarkedConcurrently(dfgCommon->weakReferences[i].get())) {
allAreLiveSoFar = false;
break;
}
@@ -2756,7 +2756,7 @@
}
if (allAreLiveSoFar) {
for (unsigned i = 0; i < dfgCommon->weakStructureReferences.size(); ++i) {
- if (!Heap::isMarked(dfgCommon->weakStructureReferences[i].get())) {
+ if (!Heap::isMarkedConcurrently(dfgCommon->weakStructureReferences[i].get())) {
allAreLiveSoFar = false;
break;
}
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/bytecode/PolymorphicAccess.cpp 2016-10-11 07:38:03 UTC (rev 207061)
@@ -555,7 +555,7 @@
switch (m_type) {
case Transition:
- if (Heap::isMarked(m_structure->previousID()))
+ if (Heap::isMarkedConcurrently(m_structure->previousID()))
visitor.appendUnbarrieredReadOnlyPointer(m_structure.get());
else
result = false;
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/Heap.h (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/Heap.h 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/Heap.h 2016-10-11 07:38:03 UTC (rev 207061)
@@ -101,6 +101,7 @@
static bool isLive(const void*);
static bool isMarked(const void*);
+ static bool isMarkedConcurrently(const void*);
static bool testAndSetMarked(HeapVersion, const void*);
static void setMarked(const void*);
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/HeapInlines.h (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/HeapInlines.h 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/HeapInlines.h 2016-10-11 07:38:03 UTC (rev 207061)
@@ -35,6 +35,7 @@
#include "Structure.h"
#include <type_traits>
#include <wtf/Assertions.h>
+#include <wtf/MainThread.h>
#include <wtf/RandomNumber.h>
namespace JSC {
@@ -76,24 +77,39 @@
inline bool Heap::isLive(const void* rawCell)
{
+ ASSERT(!mayBeGCThread());
HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
if (cell->isLargeAllocation())
return cell->largeAllocation().isLive();
MarkedBlock& block = cell->markedBlock();
- block.flipIfNecessaryConcurrently(block.vm()->heap.objectSpace().version());
+ block.flipIfNecessary(block.vm()->heap.objectSpace().version());
return block.handle().isLiveCell(cell);
}
ALWAYS_INLINE bool Heap::isMarked(const void* rawCell)
{
+ ASSERT(!mayBeGCThread());
HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
if (cell->isLargeAllocation())
return cell->largeAllocation().isMarked();
MarkedBlock& block = cell->markedBlock();
- block.flipIfNecessaryConcurrently(block.vm()->heap.objectSpace().version());
+ if (block.needsFlip(block.vm()->heap.objectSpace().version()))
+ return false;
return block.isMarked(cell);
}
+ALWAYS_INLINE bool Heap::isMarkedConcurrently(const void* rawCell)
+{
+ HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
+ if (cell->isLargeAllocation())
+ return cell->largeAllocation().isMarked();
+ MarkedBlock& block = cell->markedBlock();
+ if (block.needsFlip(block.vm()->heap.objectSpace().version()))
+ return false;
+ WTF::loadLoadFence();
+ return block.isMarked(cell);
+}
+
ALWAYS_INLINE bool Heap::testAndSetMarked(HeapVersion version, const void* rawCell)
{
HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/HeapSnapshotBuilder.cpp (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/HeapSnapshotBuilder.cpp 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/HeapSnapshotBuilder.cpp 2016-10-11 07:38:03 UTC (rev 207061)
@@ -66,7 +66,7 @@
void HeapSnapshotBuilder::appendNode(JSCell* cell)
{
ASSERT(m_profiler.activeSnapshotBuilder() == this);
- ASSERT(Heap::isMarked(cell));
+ ASSERT(Heap::isMarkedConcurrently(cell));
if (hasExistingNodeForCell(cell))
return;
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/MarkedBlock.cpp (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/MarkedBlock.cpp 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/MarkedBlock.cpp 2016-10-11 07:38:03 UTC (rev 207061)
@@ -357,7 +357,7 @@
void MarkedBlock::flipIfNecessarySlow()
{
- ASSERT(m_version != vm()->heap.objectSpace().version());
+ ASSERT(needsFlip());
clearMarks();
}
@@ -364,7 +364,7 @@
void MarkedBlock::flipIfNecessaryConcurrentlySlow()
{
LockHolder locker(m_lock);
- if (m_version != vm()->heap.objectSpace().version())
+ if (needsFlip())
clearMarks();
}
@@ -388,7 +388,7 @@
bool MarkedBlock::needsFlip()
{
- return vm()->heap.objectSpace().version() != m_version;
+ return needsFlip(vm()->heap.objectSpace().version());
}
bool MarkedBlock::Handle::needsFlip()
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/MarkedBlock.h (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/MarkedBlock.h 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/MarkedBlock.h 2016-10-11 07:38:03 UTC (rev 207061)
@@ -264,6 +264,7 @@
WeakSet& weakSet();
+ bool needsFlip(HeapVersion);
bool needsFlip();
void flipIfNecessaryConcurrently(HeapVersion);
@@ -462,15 +463,20 @@
return (reinterpret_cast<Bits>(p) - reinterpret_cast<Bits>(this)) / atomSize;
}
+inline bool MarkedBlock::needsFlip(HeapVersion heapVersion)
+{
+ return heapVersion != m_version;
+}
+
inline void MarkedBlock::flipIfNecessary(HeapVersion heapVersion)
{
- if (UNLIKELY(heapVersion != m_version))
+ if (UNLIKELY(needsFlip(heapVersion)))
flipIfNecessarySlow();
}
inline void MarkedBlock::flipIfNecessaryConcurrently(HeapVersion heapVersion)
{
- if (UNLIKELY(heapVersion != m_version))
+ if (UNLIKELY(needsFlip(heapVersion)))
flipIfNecessaryConcurrentlySlow();
WTF::loadLoadFence();
}
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/SlotVisitor.cpp (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/SlotVisitor.cpp 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/heap/SlotVisitor.cpp 2016-10-11 07:38:03 UTC (rev 207061)
@@ -227,7 +227,7 @@
template<typename ContainerType>
ALWAYS_INLINE void SlotVisitor::appendToMarkStack(ContainerType& container, JSCell* cell)
{
- ASSERT(Heap::isMarked(cell));
+ ASSERT(Heap::isMarkedConcurrently(cell));
ASSERT(!cell->isZapped());
container.noteMarked();
@@ -248,7 +248,7 @@
HeapCell* cell = bitwise_cast<HeapCell*>(base);
if (Heap::testAndSetMarked(m_version, cell)) {
- RELEASE_ASSERT(Heap::isMarked(cell));
+ RELEASE_ASSERT(Heap::isMarkedConcurrently(cell));
return;
}
@@ -293,7 +293,7 @@
ALWAYS_INLINE void SlotVisitor::visitChildren(const JSCell* cell)
{
- ASSERT(Heap::isMarked(cell));
+ ASSERT(Heap::isMarkedConcurrently(cell));
SetCurrentCellScope currentCellScope(*this, cell);
Modified: releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/runtime/Structure.cpp (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/runtime/Structure.cpp 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/_javascript_Core/runtime/Structure.cpp 2016-10-11 07:38:03 UTC (rev 207061)
@@ -1140,14 +1140,14 @@
// has any large property names.
// https://bugs.webkit.org/show_bug.cgi?id=157334
- return (!m_globalObject || Heap::isMarked(m_globalObject.get()))
- && (!storedPrototypeObject() || Heap::isMarked(storedPrototypeObject()));
+ return (!m_globalObject || Heap::isMarkedConcurrently(m_globalObject.get()))
+ && (!storedPrototypeObject() || Heap::isMarkedConcurrently(storedPrototypeObject()));
}
bool Structure::markIfCheap(SlotVisitor& visitor)
{
if (!isCheapDuringGC())
- return Heap::isMarked(this);
+ return Heap::isMarkedConcurrently(this);
visitor.appendUnbarrieredReadOnlyPointer(this);
return true;
Modified: releases/WebKitGTK/webkit-2.14/Source/WTF/ChangeLog (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/WTF/ChangeLog 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/WTF/ChangeLog 2016-10-11 07:38:03 UTC (rev 207061)
@@ -1,3 +1,15 @@
+2016-09-08 Filip Pizlo <fpi...@apple.com>
+
+ Heap::isMarked() shouldn't pay the price of concurrent lazy flipping
+ https://bugs.webkit.org/show_bug.cgi?id=161760
+
+ Reviewed by Mark Lam.
+
+ * wtf/MainThread.cpp:
+ (WTF::isMainThreadOrGCThread):
+ (WTF::mayBeGCThread):
+ * wtf/MainThread.h:
+
2016-09-06 Saam Barati <sbar...@apple.com>
Make JSMap and JSSet faster
Modified: releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/MainThread.cpp (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/MainThread.cpp 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/MainThread.cpp 2016-10-11 07:38:03 UTC (rev 207061)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2008, 2015-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -210,10 +210,15 @@
bool isMainThreadOrGCThread()
{
- if (isGCThread->isSet() && **isGCThread)
+ if (mayBeGCThread())
return true;
return isMainThread();
}
+bool mayBeGCThread()
+{
+ return isGCThread->isSet() && **isGCThread;
+}
+
} // namespace WTF
Modified: releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/MainThread.h (207060 => 207061)
--- releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/MainThread.h 2016-10-11 07:28:08 UTC (rev 207060)
+++ releases/WebKitGTK/webkit-2.14/Source/WTF/wtf/MainThread.h 2016-10-11 07:38:03 UTC (rev 207061)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2008, 2010, 2016 Apple Inc. All rights reserved.
* Copyright (C) 2007 Justin Haygood (jhayg...@reaktix.com)
*
* Redistribution and use in source and binary forms, with or without
@@ -68,6 +68,7 @@
void initializeGCThreads();
WTF_EXPORT_PRIVATE void registerGCThread();
+WTF_EXPORT_PRIVATE bool mayBeGCThread();
WTF_EXPORT_PRIVATE bool isMainThreadOrGCThread();
// NOTE: these functions are internal to the callOnMainThread implementation.
@@ -88,15 +89,16 @@
} // namespace WTF
using WTF::callOnMainThread;
-#if PLATFORM(COCOA)
-using WTF::callOnWebThreadOrDispatchAsyncOnMainThread;
-#endif
-using WTF::setMainThreadCallbacksPaused;
+using WTF::canAccessThreadLocalDataForThread;
using WTF::isMainThread;
using WTF::isMainThreadOrGCThread;
-using WTF::canAccessThreadLocalDataForThread;
using WTF::isUIThread;
using WTF::isWebThread;
+using WTF::mayBeGCThread;
+using WTF::setMainThreadCallbacksPaused;
+#if PLATFORM(COCOA)
+using WTF::callOnWebThreadOrDispatchAsyncOnMainThread;
+#endif
#if USE(WEB_THREAD)
using WTF::initializeWebThread;
using WTF::initializeApplicationUIThreadIdentifier;
