Patch forthcoming.
Created attachment 309328 [details] the patch
Comment on attachment 309328 [details] the patch Attachment 309328 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3695486 New failing tests: accessibility/accessibility-node-memory-management.html svg/animations/smil-leak-list-property-instances.svg svg/animations/smil-leak-dynamically-added-element-instances.svg fast/dom/gc-dom-tree-lifetime.html svg/animations/smil-leak-elements.svg svg/animations/smil-leak-element-instances-noBaseValRef.svg fast/workers/dedicated-worker-lifecycle.html svg/animations/smil-leak-element-instances.svg plugins/refcount-leaks.html
Created attachment 309329 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 309328 [details] the patch Attachment 309328 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3695488 New failing tests: accessibility/accessibility-node-memory-management.html svg/animations/smil-leak-list-property-instances.svg svg/animations/smil-leak-dynamically-added-element-instances.svg fast/dom/gc-dom-tree-lifetime.html svg/animations/smil-leak-elements.svg svg/animations/smil-leak-element-instances-noBaseValRef.svg fast/workers/dedicated-worker-lifecycle.html svg/animations/smil-leak-element-instances.svg
Created attachment 309330 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Those failures are "real" - I need to make the debugging GC calls in our tests do a sync GC.
Comment on attachment 309328 [details] the patch Attachment 309328 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3695552 New failing tests: svg/animations/smil-leak-list-property-instances.svg svg/animations/smil-leak-dynamically-added-element-instances.svg fast/dom/gc-dom-tree-lifetime.html svg/animations/smil-leak-elements.svg svg/animations/smil-leak-element-instances-noBaseValRef.svg fast/workers/dedicated-worker-lifecycle.html svg/animations/smil-leak-element-instances.svg
Created attachment 309332 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 309334 [details] the patch Makes tests still use sync GCs
Comment on attachment 309334 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=309334&action=review > Source/WebCore/page/MemoryRelease.cpp:99 > if (synchronous == Synchronous::Yes) { > - GCController::singleton().garbageCollectNow(); > + GCController::singleton().garbageCollectNowAsync(); This doesn't read quite right (if wantSync then doAsync). Will this work correctly when we're here because the process/app is suspending (iOS) and we have limited time to shrink as small as possible?
(In reply to Andreas Kling from comment #10) > Comment on attachment 309334 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309334&action=review > > > Source/WebCore/page/MemoryRelease.cpp:99 > > if (synchronous == Synchronous::Yes) { > > - GCController::singleton().garbageCollectNow(); > > + GCController::singleton().garbageCollectNowAsync(); > > This doesn't read quite right (if wantSync then doAsync). > Will this work correctly when we're here because the process/app is > suspending (iOS) and we have limited time to shrink as small as possible? The GC itself won't be any slower, but the runloop will yield during long GCs. The incremental sweep will be slower than the synchronous sleep, which might be a bad thing. Let's consider the terminology separately. It seems that this code uses "asynchronous" to mean something different than what it means in JSC. I think this uses asynchronous to mean that it doesn't want the GC that badly. Eventually we might want to use some other terminology here.
(In reply to Filip Pizlo from comment #11) > (In reply to Andreas Kling from comment #10) > > Comment on attachment 309334 [details] > > the patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=309334&action=review > > > > > Source/WebCore/page/MemoryRelease.cpp:99 > > > if (synchronous == Synchronous::Yes) { > > > - GCController::singleton().garbageCollectNow(); > > > + GCController::singleton().garbageCollectNowAsync(); > > > > This doesn't read quite right (if wantSync then doAsync). > > Will this work correctly when we're here because the process/app is > > suspending (iOS) and we have limited time to shrink as small as possible? > > The GC itself won't be any slower, but the runloop will yield during long > GCs. > > The incremental sweep will be slower than the synchronous sleep, which might > be a bad thing. > > Let's consider the terminology separately. It seems that this code uses > "asynchronous" to mean something different than what it means in JSC. I > think this uses asynchronous to mean that it doesn't want the GC that badly. > Eventually we might want to use some other terminology here. Thought about this more. I don't think that this patch is right. The only places where it uses the Async thing are the notifications we get that require us to GC ASAP. To do this right we'd need a much better story for sweeping.
Comment on attachment 309334 [details] the patch Removing review request to remove from review queue.