Log Message
Handling of opaque roots is wrong in EdenCollections https://bugs.webkit.org/show_bug.cgi?id=128210
Reviewed by Oliver Hunt. The set of opaque roots is always cleared during each collection. We should instead persist the set of opaque roots across EdenCollections and only clear it at the beginning of FullCollections. Also added a couple of custom objects to the jsc shell that allow us to test this. * heap/GCThreadSharedData.cpp: (JSC::GCThreadSharedData::reset): (JSC::GCThreadSharedData::didStartMarking): * heap/Heap.cpp: (JSC::Heap::markRoots): * heap/Heap.h: (JSC::Heap::setShouldDoFullCollection): * heap/SlotVisitor.cpp: (JSC::SlotVisitor::didStartMarking): (JSC::SlotVisitor::reset): * heap/SlotVisitor.h: * jsc.cpp: (WTF::Element::Element): (WTF::Element::root): (WTF::Element::setRoot): (WTF::Element::create): (WTF::Element::createStructure): (WTF::ElementHandleOwner::isReachableFromOpaqueRoots): (WTF::Root::Root): (WTF::Root::element): (WTF::Root::setElement): (WTF::Root::create): (WTF::Root::createStructure): (WTF::Root::visitChildren): (WTF::Element::handleOwner): (WTF::Element::finishCreation): (GlobalObject::finishCreation): (functionCreateRoot): (functionCreateElement): (functionGetElement): (functionSetElementRoot): (functionGCAndSweep): (functionFullGC): (functionEdenGC):
Modified Paths
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/heap/GCThreadSharedData.cpp
- trunk/Source/_javascript_Core/heap/Heap.cpp
- trunk/Source/_javascript_Core/heap/Heap.h
- trunk/Source/_javascript_Core/heap/SlotVisitor.cpp
- trunk/Source/_javascript_Core/heap/SlotVisitor.h
- trunk/Source/_javascript_Core/jsc.cpp
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (163506 => 163507)
--- trunk/Source/_javascript_Core/ChangeLog 2014-02-06 03:07:17 UTC (rev 163506)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-02-06 03:38:19 UTC (rev 163507)
@@ -1,3 +1,50 @@
+2014-02-05 Mark Hahnenberg <[email protected]>
+
+ Handling of opaque roots is wrong in EdenCollections
+ https://bugs.webkit.org/show_bug.cgi?id=128210
+
+ Reviewed by Oliver Hunt.
+
+ The set of opaque roots is always cleared during each collection. We should instead persist
+ the set of opaque roots across EdenCollections and only clear it at the beginning of FullCollections.
+
+ Also added a couple of custom objects to the jsc shell that allow us to test this.
+
+ * heap/GCThreadSharedData.cpp:
+ (JSC::GCThreadSharedData::reset):
+ (JSC::GCThreadSharedData::didStartMarking):
+ * heap/Heap.cpp:
+ (JSC::Heap::markRoots):
+ * heap/Heap.h:
+ (JSC::Heap::setShouldDoFullCollection):
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::didStartMarking):
+ (JSC::SlotVisitor::reset):
+ * heap/SlotVisitor.h:
+ * jsc.cpp:
+ (WTF::Element::Element):
+ (WTF::Element::root):
+ (WTF::Element::setRoot):
+ (WTF::Element::create):
+ (WTF::Element::createStructure):
+ (WTF::ElementHandleOwner::isReachableFromOpaqueRoots):
+ (WTF::Root::Root):
+ (WTF::Root::element):
+ (WTF::Root::setElement):
+ (WTF::Root::create):
+ (WTF::Root::createStructure):
+ (WTF::Root::visitChildren):
+ (WTF::Element::handleOwner):
+ (WTF::Element::finishCreation):
+ (GlobalObject::finishCreation):
+ (functionCreateRoot):
+ (functionCreateElement):
+ (functionGetElement):
+ (functionSetElementRoot):
+ (functionGCAndSweep):
+ (functionFullGC):
+ (functionEdenGC):
+
2014-02-05 Anders Carlsson <[email protected]>
Remove unused functions.
Modified: trunk/Source/_javascript_Core/heap/GCThreadSharedData.cpp (163506 => 163507)
--- trunk/Source/_javascript_Core/heap/GCThreadSharedData.cpp 2014-02-06 03:07:17 UTC (rev 163506)
+++ trunk/Source/_javascript_Core/heap/GCThreadSharedData.cpp 2014-02-06 03:38:19 UTC (rev 163507)
@@ -118,16 +118,11 @@
}
#endif
}
-
+
void GCThreadSharedData::reset()
{
ASSERT(m_sharedMarkStack.isEmpty());
-#if ENABLE(PARALLEL_GC)
- m_opaqueRoots.clear();
-#else
- ASSERT(m_opaqueRoots.isEmpty());
-#endif
m_weakReferenceHarvesters.removeAll();
if (m_shouldHashCons) {
@@ -158,6 +153,13 @@
void GCThreadSharedData::didStartMarking()
{
+ if (m_vm->heap.operationInProgress() == FullCollection) {
+#if ENABLE(PARALLEL_GC)
+ m_opaqueRoots.clear();
+#else
+ ASSERT(m_opaqueRoots.isEmpty());
+#endif
+}
std::lock_guard<std::mutex> lock(m_markingMutex);
m_parallelMarkersShouldExit = false;
startNextPhase(Mark);
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (163506 => 163507)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2014-02-06 03:07:17 UTC (rev 163506)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2014-02-06 03:38:19 UTC (rev 163507)
@@ -490,7 +490,7 @@
m_sharedData.didStartMarking();
SlotVisitor& visitor = m_slotVisitor;
- visitor.setup();
+ visitor.didStartMarking();
HeapRootVisitor heapRootVisitor(visitor);
#if ENABLE(GGC)
Modified: trunk/Source/_javascript_Core/heap/Heap.h (163506 => 163507)
--- trunk/Source/_javascript_Core/heap/Heap.h 2014-02-06 03:07:17 UTC (rev 163506)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2014-02-06 03:38:19 UTC (rev 163507)
@@ -148,7 +148,8 @@
JS_EXPORT_PRIVATE void collectAllGarbage();
bool shouldCollect();
void gcTimerDidFire() { m_shouldDoFullCollection = true; }
- void collect();
+ void setShouldDoFullCollection(bool shouldDoFullCollection) { m_shouldDoFullCollection = shouldDoFullCollection; }
+ JS_EXPORT_PRIVATE void collect();
bool collectIfNecessaryOrDefer(); // Returns true if it did collect.
void reportExtraMemoryCost(size_t cost);
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.cpp (163506 => 163507)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2014-02-06 03:07:17 UTC (rev 163506)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2014-02-06 03:38:19 UTC (rev 163507)
@@ -36,8 +36,16 @@
clearMarkStack();
}
-void SlotVisitor::setup()
+void SlotVisitor::didStartMarking()
{
+ if (heap()->operationInProgress() == FullCollection) {
+#if ENABLE(PARALLEL_GC)
+ ASSERT(m_opaqueRoots.isEmpty()); // Should have merged by now.
+#else
+ m_opaqueRoots.clear();
+#endif
+ }
+
m_shared.m_shouldHashCons = m_shared.m_vm->haveEnoughNewStringsToHashCons();
m_shouldHashCons = m_shared.m_shouldHashCons;
#if ENABLE(PARALLEL_GC)
@@ -52,11 +60,6 @@
m_bytesCopied = 0;
m_visitCount = 0;
ASSERT(m_stack.isEmpty());
-#if ENABLE(PARALLEL_GC)
- ASSERT(m_opaqueRoots.isEmpty()); // Should have merged by now.
-#else
- m_opaqueRoots.clear();
-#endif
if (m_shouldHashCons) {
m_uniqueStrings.clear();
m_shouldHashCons = false;
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.h (163506 => 163507)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.h 2014-02-06 03:07:17 UTC (rev 163506)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.h 2014-02-06 03:38:19 UTC (rev 163507)
@@ -78,7 +78,7 @@
GCThreadSharedData& sharedData() const { return m_shared; }
bool isEmpty() { return m_stack.isEmpty(); }
- void setup();
+ void didStartMarking();
void reset();
void clearMarkStack();
Modified: trunk/Source/_javascript_Core/jsc.cpp (163506 => 163507)
--- trunk/Source/_javascript_Core/jsc.cpp 2014-02-06 03:07:17 UTC (rev 163506)
+++ trunk/Source/_javascript_Core/jsc.cpp 2014-02-06 03:38:19 UTC (rev 163507)
@@ -92,13 +92,136 @@
using namespace JSC;
using namespace WTF;
+namespace {
+
+class Element;
+class ElementHandleOwner;
+class Root;
+
+class Element : public JSNonFinalObject {
+public:
+ Element(VM& vm, Structure* structure, Root* root)
+ : Base(vm, structure)
+ , m_root(root)
+ {
+ }
+
+ typedef JSNonFinalObject Base;
+ static const bool needsDestruction = false;
+
+ Root* root() const { return m_root; }
+ void setRoot(Root* root) { m_root = root; }
+
+ static Element* create(VM& vm, JSGlobalObject* globalObject, Root* root)
+ {
+ Structure* structure = createStructure(vm, globalObject, jsNull());
+ Element* element = new (NotNull, allocateCell<Element>(vm.heap, sizeof(Element))) Element(vm, structure, root);
+ element->finishCreation(vm);
+ return element;
+ }
+
+ void finishCreation(VM&);
+
+ static ElementHandleOwner* handleOwner();
+
+ static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
+ {
+ return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
+ }
+
+ DECLARE_INFO;
+
+private:
+ Root* m_root;
+};
+
+class ElementHandleOwner : public WeakHandleOwner {
+public:
+ virtual bool isReachableFromOpaqueRoots(Handle<JSC::Unknown> handle, void*, SlotVisitor& visitor)
+ {
+ Element* element = jsCast<Element*>(handle.slot()->asCell());
+ return visitor.containsOpaqueRoot(element->root());
+ }
+};
+
+class Root : public JSDestructibleObject {
+public:
+ Root(VM& vm, Structure* structure)
+ : Base(vm, structure)
+ {
+ }
+
+ Element* element()
+ {
+ return m_element.get();
+ }
+
+ void setElement(Element* element)
+ {
+ Weak<Element> newElement(element, Element::handleOwner());
+ m_element.swap(newElement);
+ }
+
+ static Root* create(VM& vm, JSGlobalObject* globalObject)
+ {
+ Structure* structure = createStructure(vm, globalObject, jsNull());
+ Root* root = new (NotNull, allocateCell<Root>(vm.heap, sizeof(Root))) Root(vm, structure);
+ root->finishCreation(vm);
+ return root;
+ }
+
+ typedef JSDestructibleObject Base;
+
+ DECLARE_INFO;
+ static const bool needsDestruction = true;
+
+ static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
+ {
+ return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
+ }
+
+ static void visitChildren(JSCell* thisObject, SlotVisitor& visitor)
+ {
+ Base::visitChildren(thisObject, visitor);
+ visitor.addOpaqueRoot(thisObject);
+ }
+
+private:
+ Weak<Element> m_element;
+};
+
+const ClassInfo Element::s_info = { "Element", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(Element) };
+const ClassInfo Root::s_info = { "Root", &Base::s_info, 0, 0, CREATE_METHOD_TABLE(Root) };
+
+ElementHandleOwner* Element::handleOwner()
+{
+ static ElementHandleOwner* owner = 0;
+ if (!owner)
+ owner = new ElementHandleOwner();
+ return owner;
+}
+
+void Element::finishCreation(VM& vm)
+{
+ Base::finishCreation(vm);
+ m_root->setElement(this);
+}
+
+}
+
static bool fillBufferWithContentsOfFile(const String& fileName, Vector<char>& buffer);
+static EncodedJSValue JSC_HOST_CALL functionSetElementRoot(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionCreateRoot(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionCreateElement(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionGetElement(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionPrint(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionDebug(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionDescribe(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionJSCStack(ExecState*);
-static EncodedJSValue JSC_HOST_CALL functionGC(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionGCAndSweep(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionFullGC(ExecState*);
+static EncodedJSValue JSC_HOST_CALL functionEdenGC(ExecState*);
#ifndef NDEBUG
static EncodedJSValue JSC_HOST_CALL functionReleaseExecutableMemory(ExecState*);
static EncodedJSValue JSC_HOST_CALL functionDumpCallFrame(ExecState*);
@@ -219,7 +342,9 @@
addFunction(vm, "describe", functionDescribe, 1);
addFunction(vm, "print", functionPrint, 1);
addFunction(vm, "quit", functionQuit, 0);
- addFunction(vm, "gc", functionGC, 0);
+ addFunction(vm, "gc", functionGCAndSweep, 0);
+ addFunction(vm, "fullGC", functionFullGC, 0);
+ addFunction(vm, "edenGC", functionEdenGC, 0);
#ifndef NDEBUG
addFunction(vm, "dumpCallFrame", functionDumpCallFrame, 0);
addFunction(vm, "releaseExecutableMemory", functionReleaseExecutableMemory, 0);
@@ -241,6 +366,10 @@
addFunction(vm, "setSamplingFlags", functionSetSamplingFlags, 1);
addFunction(vm, "clearSamplingFlags", functionClearSamplingFlags, 1);
#endif
+ addConstructableFunction(vm, "Root", functionCreateRoot, 0);
+ addConstructableFunction(vm, "Element", functionCreateElement, 1);
+ addFunction(vm, "getElement", functionGetElement, 1);
+ addFunction(vm, "setElementRoot", functionSetElementRoot, 2);
JSArray* array = constructEmptyArray(globalExec(), 0);
for (size_t i = 0; i < arguments.size(); ++i)
@@ -357,13 +486,60 @@
return JSValue::encode(jsUndefined());
}
-EncodedJSValue JSC_HOST_CALL functionGC(ExecState* exec)
+EncodedJSValue JSC_HOST_CALL functionCreateRoot(ExecState* exec)
{
JSLockHolder lock(exec);
+ return JSValue::encode(Root::create(exec->vm(), exec->lexicalGlobalObject()));
+}
+
+EncodedJSValue JSC_HOST_CALL functionCreateElement(ExecState* exec)
+{
+ JSLockHolder lock(exec);
+ JSValue arg = exec->argument(0);
+ return JSValue::encode(Element::create(exec->vm(), exec->lexicalGlobalObject(), arg.isNull() ? nullptr : jsCast<Root*>(exec->argument(0))));
+}
+
+EncodedJSValue JSC_HOST_CALL functionGetElement(ExecState* exec)
+{
+ JSLockHolder lock(exec);
+ Element* result = jsCast<Root*>(exec->argument(0).asCell())->element();
+ return JSValue::encode(result ? result : jsUndefined());
+}
+
+EncodedJSValue JSC_HOST_CALL functionSetElementRoot(ExecState* exec)
+{
+ JSLockHolder lock(exec);
+ Element* element = jsCast<Element*>(exec->argument(0));
+ Root* root = jsCast<Root*>(exec->argument(1));
+ element->setRoot(root);
+ return JSValue::encode(jsUndefined());
+}
+
+EncodedJSValue JSC_HOST_CALL functionGCAndSweep(ExecState* exec)
+{
+ JSLockHolder lock(exec);
exec->heap()->collectAllGarbage();
return JSValue::encode(jsUndefined());
}
+EncodedJSValue JSC_HOST_CALL functionFullGC(ExecState* exec)
+{
+ JSLockHolder lock(exec);
+ Heap* heap = exec->heap();
+ heap->setShouldDoFullCollection(true);
+ exec->heap()->collect();
+ return JSValue::encode(jsUndefined());
+}
+
+EncodedJSValue JSC_HOST_CALL functionEdenGC(ExecState* exec)
+{
+ JSLockHolder lock(exec);
+ Heap* heap = exec->heap();
+ heap->setShouldDoFullCollection(false);
+ heap->collect();
+ return JSValue::encode(jsUndefined());
+}
+
#ifndef NDEBUG
EncodedJSValue JSC_HOST_CALL functionReleaseExecutableMemory(ExecState* exec)
{
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
