Bug 170690 – WebAssembly: report Memory usage to GC

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[JF Bastien]

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.

[Radar WebKit Bug Importer]

<rdar://problem/31965310>

[JF Bastien]

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.

[Keith Miller]

Comment on attachment 309699 [details]
patch

r=me.

[WebKit Commit Bot]

Comment on attachment 309699 [details]
patch

Clearing flags on attachment: 309699

Committed r216911: <http://trac.webkit.org/changeset/216911>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.