Bug 170690 - WebAssembly: report Memory usage to GC
Summary: WebAssembly: report Memory usage to GC
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: JF Bastien
Keywords: InRadar
Depends on: 170628
Blocks: 159775
  Show dependency treegraph
Reported: 2017-04-10 13:42 PDT by JF Bastien
Modified: 2017-05-16 00:10 PDT (History)
8 users (show)

See Also:

patch (2.80 KB, patch)
2017-05-11 00:32 PDT, JF Bastien
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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

void JSWebAssemblyMemory::finishCreation(VM& vm)

JSWebAssemblyMemory::visitChildren(JSCell* cell, SlotVisitor& visitor)

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 1 Radar WebKit Bug Importer 2017-05-03 09:54:31 PDT
Comment 2 JF Bastien 2017-05-11 00:32:32 PDT
Created attachment 309699 [details]

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 3 Keith Miller 2017-05-15 23:56:19 PDT
Comment on attachment 309699 [details]

Comment 4 WebKit Commit Bot 2017-05-16 00:09:59 PDT
Comment on attachment 309699 [details]

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.