Patch coming.
<rdar://problem/30451496>
Created attachment 301396 [details] proposed patch. Let's get some EWS testing on this.
Comment on attachment 301396 [details] proposed patch. Looks ready for a review.
Comment on attachment 301396 [details] proposed patch. r=me
Comment on attachment 301396 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=301396&action=review > Source/JavaScriptCore/heap/Heap.cpp:1060 > + sweepSynchronously(); > + if (Options::useZombieMode()) > + zombifyDeadObjects(); Turns out this is not needed because collectSync() above will take care of invoking sweepSynchronously() and zombifyDeadObjects() if needed. I'll also add some GC logging when zombifying. Updated patch coming soon.
Comment on attachment 301396 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=301396&action=review >> Source/JavaScriptCore/heap/Heap.cpp:1060 >> + zombifyDeadObjects(); > > Turns out this is not needed because collectSync() above will take care of invoking sweepSynchronously() and zombifyDeadObjects() if needed. I'll also add some GC logging when zombifying. Updated patch coming soon. Correction: sweepSyncronously is still needed if neither Options::sweepSynchronously() or Options::useZombieMode() are set because collectSync() does not sweep synchronously by default. Will fix.
Created attachment 301428 [details] proposed patch with fixes.
Comment on attachment 301428 [details] proposed patch with fixes. View in context: https://bugs.webkit.org/attachment.cgi?id=301428&action=review LGTM > Source/JavaScriptCore/heap/Heap.cpp:1043 > + dataLog("=> ", capacity() / 1024, "kb, ", after - before, "ms] "); Why not put the "\n" here? > Source/JavaScriptCore/heap/Heap.cpp:1980 > + dataLog("=> ", capacity() / 1024, "kb, ", after - before, "ms] "); ditto
Comment on attachment 301428 [details] proposed patch with fixes. View in context: https://bugs.webkit.org/attachment.cgi?id=301428&action=review Thanks for the review. >> Source/JavaScriptCore/heap/Heap.cpp:1043 >> + dataLog("=> ", capacity() / 1024, "kb, ", after - before, "ms] "); > > Why not put the "\n" here? Because this function may be called from Heap::finalize() which has its own logging which wraps this line. Putting a '\n' here will premature terminate the "[GC: finalize ...]" logging in Heap::finalize(). >> Source/JavaScriptCore/heap/Heap.cpp:1980 >> + dataLog("=> ", capacity() / 1024, "kb, ", after - before, "ms] "); > > ditto Same as above.
Created attachment 301526 [details] proposed patch. One more time: this time, re-define useZombieMode as the combination of sweepSynchronously + scribbleFreeCells.
Thanks for the reviews. Landed in r212310: <http://trac.webkit.org/r212310>.