Title: [121316] trunk/Source
- Revision
- 121316
- Author
- [email protected]
- Date
- 2012-06-26 20:44:05 -0700 (Tue, 26 Jun 2012)
Log Message
Reduced (but did not eliminate) use of "berzerker GC"
https://bugs.webkit.org/show_bug.cgi?id=89237
Reviewed by Gavin Barraclough.
(PART 2)
../_javascript_Core:
This part turns off "berzerker GC" and turns on incremental shrinking.
* heap/IncrementalSweeper.cpp:
(JSC::IncrementalSweeper::doSweep): Free or shrink after sweeping to
maintain the behavior we used to get from the occasional berzerker GC,
which would run all finalizers and then free or shrink all blocks
synchronously.
* heap/MarkedBlock.h:
(JSC::MarkedBlock::needsSweeping): Sweep zapped blocks, too. It's always
safe to sweep a zapped block (that's the point of zapping), and it's
sometimes profitable. For example, consider this case: Block A does some
allocation (transitioning Block A from Marked to FreeListed), then GC
happens (transitioning Block A to Zapped), then all objects in Block A
are free, then the incremental sweeper visits Block A. If we skipped
Zapped blocks, we'd skip Block A, even though it would be profitable to
run its destructors and free its memory.
* runtime/GCActivityCallback.cpp:
(JSC::DefaultGCActivityCallback::doWork): Don't sweep eagerly; we'll do
this incrementally.
../WebCore:
Don't ASSERT that RootObject's destructor runs and invalidates all
RuntimeObjects before their destructors run.
We don't guarantee this behavior because some RuntimeObjects may already
be garbage by the time RootObject's destructor runs, in which case
RootObject's weak pointers will be NULL, and RootObject will not call
invalidate() on them.
It's been theoretically possible for this ASSERT to fire for a while now.
This patch makes it fire all the time.
Luckily, we only needed the behavior guarded by this ASSERT for WebKit1
in Safari on Windows (cf. https://bugs.webkit.org/show_bug.cgi?id=61317),
to handle the way WebKit1 would unload plugin DLLs. If this ever becomes
an issue again, we can fix it by (a) not unloading plugin DLLs,
(b) migrating WebKit1 to the WebKit2 JS-plugin binding model, (c) making
the Instance pointer in a RuntimeObject an indirect pointer through
RootObject, or (c) giving RuntimeObject some sort of special way to
access a zombie weak pointer.
* bridge/runtime_object.cpp:
(JSC::Bindings::RuntimeObject::destroy): ASSERT removed. Anders said so.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (121315 => 121316)
--- trunk/Source/_javascript_Core/ChangeLog 2012-06-27 03:30:32 UTC (rev 121315)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-06-27 03:44:05 UTC (rev 121316)
@@ -1,3 +1,34 @@
+2012-06-26 Geoffrey Garen <[email protected]>
+
+ Reduced (but did not eliminate) use of "berzerker GC"
+ https://bugs.webkit.org/show_bug.cgi?id=89237
+
+ Reviewed by Gavin Barraclough.
+
+ (PART 2)
+
+ This part turns off "berzerker GC" and turns on incremental shrinking.
+
+ * heap/IncrementalSweeper.cpp:
+ (JSC::IncrementalSweeper::doSweep): Free or shrink after sweeping to
+ maintain the behavior we used to get from the occasional berzerker GC,
+ which would run all finalizers and then free or shrink all blocks
+ synchronously.
+
+ * heap/MarkedBlock.h:
+ (JSC::MarkedBlock::needsSweeping): Sweep zapped blocks, too. It's always
+ safe to sweep a zapped block (that's the point of zapping), and it's
+ sometimes profitable. For example, consider this case: Block A does some
+ allocation (transitioning Block A from Marked to FreeListed), then GC
+ happens (transitioning Block A to Zapped), then all objects in Block A
+ are free, then the incremental sweeper visits Block A. If we skipped
+ Zapped blocks, we'd skip Block A, even though it would be profitable to
+ run its destructors and free its memory.
+
+ * runtime/GCActivityCallback.cpp:
+ (JSC::DefaultGCActivityCallback::doWork): Don't sweep eagerly; we'll do
+ this incrementally.
+
2012-06-26 Filip Pizlo <[email protected]>
DFG PutByValAlias is too aggressive
Modified: trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp (121315 => 121316)
--- trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp 2012-06-27 03:30:32 UTC (rev 121315)
+++ trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp 2012-06-27 03:44:05 UTC (rev 121316)
@@ -78,6 +78,7 @@
continue;
block->sweep();
+ m_globalData->heap.objectSpace().freeOrShrinkBlock(block);
CFTimeInterval elapsedTime = WTF::monotonicallyIncreasingTime() - sweepBeginTime;
if (elapsedTime < sweepTimeSlice)
Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.h (121315 => 121316)
--- trunk/Source/_javascript_Core/heap/MarkedBlock.h 2012-06-27 03:30:32 UTC (rev 121315)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.h 2012-06-27 03:44:05 UTC (rev 121316)
@@ -408,7 +408,7 @@
inline bool MarkedBlock::needsSweeping()
{
- return m_state == Marked;
+ return m_state == Marked || m_state == Zapped;
}
#if ENABLE(GGC)
Modified: trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp (121315 => 121316)
--- trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp 2012-06-27 03:30:32 UTC (rev 121315)
+++ trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp 2012-06-27 03:44:05 UTC (rev 121316)
@@ -75,7 +75,7 @@
return;
}
#endif
- heap->collectAllGarbage();
+ heap->collect(Heap::DoNotSweep);
}
void DefaultGCActivityCallback::scheduleTimer(double newDelay)
Modified: trunk/Source/WebCore/ChangeLog (121315 => 121316)
--- trunk/Source/WebCore/ChangeLog 2012-06-27 03:30:32 UTC (rev 121315)
+++ trunk/Source/WebCore/ChangeLog 2012-06-27 03:44:05 UTC (rev 121316)
@@ -1,3 +1,35 @@
+2012-06-26 Geoffrey Garen <[email protected]>
+
+ Reduced (but did not eliminate) use of "berzerker GC"
+ https://bugs.webkit.org/show_bug.cgi?id=89237
+
+ Reviewed by Gavin Barraclough.
+
+ (PART 2)
+
+ Don't ASSERT that RootObject's destructor runs and invalidates all
+ RuntimeObjects before their destructors run.
+
+ We don't guarantee this behavior because some RuntimeObjects may already
+ be garbage by the time RootObject's destructor runs, in which case
+ RootObject's weak pointers will be NULL, and RootObject will not call
+ invalidate() on them.
+
+ It's been theoretically possible for this ASSERT to fire for a while now.
+ This patch makes it fire all the time.
+
+ Luckily, we only needed the behavior guarded by this ASSERT for WebKit1
+ in Safari on Windows (cf. https://bugs.webkit.org/show_bug.cgi?id=61317),
+ to handle the way WebKit1 would unload plugin DLLs. If this ever becomes
+ an issue again, we can fix it by (a) not unloading plugin DLLs,
+ (b) migrating WebKit1 to the WebKit2 JS-plugin binding model, (c) making
+ the Instance pointer in a RuntimeObject an indirect pointer through
+ RootObject, or (c) giving RuntimeObject some sort of special way to
+ access a zombie weak pointer.
+
+ * bridge/runtime_object.cpp:
+ (JSC::Bindings::RuntimeObject::destroy): ASSERT removed. Anders said so.
+
2012-06-26 Douglas Stockwell <[email protected]>
REGRESSION(r107836): box shadow not drawn for opaque images with an opaque background
Modified: trunk/Source/WebCore/bridge/runtime_object.cpp (121315 => 121316)
--- trunk/Source/WebCore/bridge/runtime_object.cpp 2012-06-27 03:30:32 UTC (rev 121315)
+++ trunk/Source/WebCore/bridge/runtime_object.cpp 2012-06-27 03:44:05 UTC (rev 121316)
@@ -51,9 +51,7 @@
void RuntimeObject::destroy(JSCell* cell)
{
- RuntimeObject* thisObject = static_cast<RuntimeObject*>(cell);
- ASSERT(!thisObject->m_instance);
- thisObject->RuntimeObject::~RuntimeObject();
+ static_cast<RuntimeObject*>(cell)->RuntimeObject::~RuntimeObject();
}
void RuntimeObject::invalidate()
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
