Bug 169704

Created attachment 304563 [details] patch

Created attachment 304564 [details] patch

Comment on attachment 304564 [details] patch r=me

Comment on attachment 304564 [details] patch Clearing flags on attachment: 304564 Committed r214018: <http://trac.webkit.org/changeset/214018>

All reviewed patches have been landed. Closing bug.

Comment on attachment 304564 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=304564&action=review > Source/JavaScriptCore/wasm/WasmMemory.cpp:123 > - vm.heap.collectSync(); > + vm.heap.collectAllGarbage(); > return dequeFastMemory(); This is sad - isn't this whole thing part of some promise anyway? Seems like waiting for the GC to finish should be part of the promise.

(In reply to comment #6) > Comment on attachment 304564 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=304564&action=review > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:123 > > - vm.heap.collectSync(); > > + vm.heap.collectAllGarbage(); > > return dequeFastMemory(); > > This is sad - isn't this whole thing part of some promise anyway? Seems > like waiting for the GC to finish should be part of the promise. I see what you're getting at here, but I think it's somewhat tricky to do. We would need a two-level strategy. Call collectSync, then set up a promise to be called when things get swept, then set up another promise when we're done compiling. I'm not sure exactly how to do that. It might be worth doing as part of Keith's work on async compilation, but we also don't expect this code to run very often.

(In reply to comment #7) > (In reply to comment #6) > > Comment on attachment 304564 [details] > > patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=304564&action=review > > > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:123 > > > - vm.heap.collectSync(); > > > + vm.heap.collectAllGarbage(); > > > return dequeFastMemory(); > > > > This is sad - isn't this whole thing part of some promise anyway? Seems > > like waiting for the GC to finish should be part of the promise. > > I see what you're getting at here, but I think it's somewhat tricky to do. > We would need a two-level strategy. Call collectSync, then set up a promise > to be called when things get swept, then set up another promise when we're > done compiling. I'm not sure exactly how to do that. It might be worth doing > as part of Keith's work on async compilation, but we also don't expect this > code to run very often. Right, we would need the incremental sweeper to run more aggressively and we would want it to notify us when done. Worth filing a bug.

(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > Comment on attachment 304564 [details] > > > patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=304564&action=review > > > > > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:123 > > > > - vm.heap.collectSync(); > > > > + vm.heap.collectAllGarbage(); > > > > return dequeFastMemory(); > > > > > > This is sad - isn't this whole thing part of some promise anyway? Seems > > > like waiting for the GC to finish should be part of the promise. > > > > I see what you're getting at here, but I think it's somewhat tricky to do. > > We would need a two-level strategy. Call collectSync, then set up a promise > > to be called when things get swept, then set up another promise when we're > > done compiling. I'm not sure exactly how to do that. It might be worth doing > > as part of Keith's work on async compilation, but we also don't expect this > > code to run very often. > > Right, we would need the incremental sweeper to run more aggressively and we > would want it to notify us when done. > > Worth filing a bug. Filed: https://bugs.webkit.org/show_bug.cgi?id=169728