|Summary:||WebAssembly: report Memory usage to GC|
|Product:||WebKit||Reporter:||JF Bastien <jfbastien>|
|Severity:||Normal||CC:||commit-queue, fpizlo, jfbastien, keith_miller, mark.lam, msaboff, sbarati, webkit-bug-importer|
|Version:||WebKit Nightly Build|
|Bug Depends on:||170628|
Description JF Bastien 2017-04-10 13:42:42 PDT
As part of bug #170628 I noticed that we never report the memory allocated by WebAssembly.Memory to the GC. This change would do it: Wasm::PageCount JSWebAssemblyMemory::grow(VM& vm, ExecState* exec, uint32_t delt ... vm.heap.reportExtraMemoryAllocated(Wasm::PageCount(delta).bytes()); void JSWebAssemblyMemory::finishCreation(VM& vm) ... heap()->reportExtraMemoryAllocated(m_memory->size()); JSWebAssemblyMemory::visitChildren(JSCell* cell, SlotVisitor& visitor) ... visitor.reportExtraMemoryVisited(thisObject->memory().size()); I'm not sure it's the right thing to do though! The "initial" size seems like a good thing to report, but if we haven't actually backed it up with physical pages then it may severely over-estimate allocation and cause the GC to run too much. This change is significant enough that I'll punt it to its own patch, after bug #170628.
Comment 2 JF Bastien 2017-05-11 00:32:32 PDT
Created attachment 309699 [details] patch I'm still not sure this is the right thing. We definitely should tell the GC about this memory but: 1. It's more memory than other allocations. Maybe that's the same as ArrayBuffer and doesn't matter? 2. We're not sure all pages are dirty, so we could be over-reporting if the developer over-allocates. We just don't know if they do. I think these don't matter, and we can adjust if they do, so this patch is probably right. I'd nonetheless would rather ask and get feedback on these points.
Comment 4 WebKit Commit Bot 2017-05-16 00:09:59 PDT
Comment on attachment 309699 [details] patch Clearing flags on attachment: 309699 Committed r216911: <http://trac.webkit.org/changeset/216911>
Comment 5 WebKit Commit Bot 2017-05-16 00:10:02 PDT
All reviewed patches have been landed. Closing bug.