546477 - report JS engine VirtualAlloc usage (for the stack) to about:memory

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

While asking Vlad about VirtualAlloc on Windows, he told me that we need to be reporting our memory usage that skips the system allocator to about:memory.  Bug 540706 adds a new use of VirtualAlloc, but there are other existing uses in the JS engine.

Comment 1

[Mike Shaver (:shaver emeritus)]

This sounds like bug 532486, though that is a bug with two purposes (count, and switch to valloc).  Maybe we should narrow 532486 to the valloc case, and make this one for counting?

Comment 2

[Igor Bukanov]

(In reply to comment #1)
> This sounds like bug 532486, though that is a bug with two purposes (count, and
> switch to valloc).  Maybe we should narrow 532486 to the valloc case, and make
> this one for counting?

The bug 557538 would allow to do all the necessary accounting outside the engine. And with jemalloc available together with the bug 553812 it would allow to have a common pool of chunks for jemalloc and the GC.

Comment 3

[Nicholas Nethercote [inactive]]

Luke, are the VirtualAlloc calls in question the ones for code generated by the JITs?  If so, bug 633653 has this covered.

Comment 4

[Luke Wagner [:luke]]

The VirtualAlloc calls I was referring to are in StackSpace::init and StackSpace::bumpCommit.

Comment 5

[Nicholas Nethercote [inactive]]

> The VirtualAlloc calls I was referring to are in StackSpace::init and
> StackSpace::bumpCommit.

StackSpace::init allocates a 4MB chunk. Can you explain why Windows needs bumpCommit, and what it does?

The other place in the JS engine that calls mmap/VirtualAlloc is in jsgcchunk.cpp.  That only happens if MOZ_MEMORY is undefined and it's covered by the js/gc-heap reporter in xpcjsruntime.cpp anyway.

Comment 6

[Nicholas Nethercote [inactive]]

Luke, is determining the StackSpace memory usage as simple as using ThreadDataIter to count how many ThreadData objects there are, and multiplying that number by StackSpace::CAPACITY_BYTES?

Comment 7

[Luke Wagner [:luke]]

For Winodws, StackSpace::init reserves 4MB (via MEM_RESERVE) but initially commits (via MEM_COMMIT) only StackSpace::COMMIT_BYTES (128K) and grows as necessary (in bumpCommit).  (Note that this notion of "commit" is different than the kernel's.)  Thus, on windows, only the committed memory will be included in virtual memory usage.  This can be measured via (StackSpace::commitEnd_ - StackSpace::base_) * sizeof(Value).  Feel free to add a public memoryUsed() helper to return this.

On Linux/OSX, I initially wasn't able to find an equivalent of MEM_RESERVE/MEM_COMMIT, so we just mmap the whole thing eagerly.  However, I just learned (like yesterday) you can achieve the effect of MEM_RESERVE (without MEM_COMMIT) by mmap()ing with PROT_NONE and using mprotect() to later commit the memory.  I haven't verified this solution though.

Comment 8

[Nicholas Nethercote [inactive]]

See also bug 656941, which proposed to extend the current Windows-only behaviour to all platforms.

Comment 9

[Nicholas Nethercote [inactive]]

Attachment: patch

This adds two reporters:  explicit/js/stack-space-committed and explicit/js/stack-space-uncommitted.  I don't have descriptions for them in yet, hence the "njn" comments.  Luke, does having two reporters seem reasonable to you?  The uncommitted one is currently always zero on non-Windows platforms.

Comment 10

[Luke Wagner [:luke]]

Comment on attachment 536826 [details] [diff] [review]
patch

I don't understand the NS_MEMORY_REPORTER_IMPLEMENT macro, but it looks like uncommitted is going to be included in the "Explicit Allocations" sum.  Now, I know uncommitted memory still counts against us since it uses up VM address space which can crash us on 32-bit systems, but perhaps we could instead give "reserved but uncommitted" as a separate item under "Other Measurements".  (Will "vsize" include "reserved but uncommitted"?  If so, then it could be a subitem.)

Comment 11

[Nicholas Nethercote [inactive]]

(In reply to comment #10)
> Comment on attachment 536826 [details] [diff] [review] [review]
> patch
> 
> I don't understand the NS_MEMORY_REPORTER_IMPLEMENT macro

http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsIMemoryReporter.idl#135

> but it looks like
> uncommitted is going to be included in the "Explicit Allocations" sum.  Now,
> I know uncommitted memory still counts against us since it uses up VM
> address space which can crash us on 32-bit systems, but perhaps we could
> instead give "reserved but uncommitted" as a separate item under "Other
> Measurements".  (Will "vsize" include "reserved but uncommitted"?  If so,
> then it could be a subitem.)

"vsize" will include reserved but uncommitted memory, but it's not present on Windows;  Windows has "private" instead.  And about:memory doesn't allow for "Other measurements" items to have sub-items, it's just a flat list.

Probably the best thing to do is to just remove stack-space-uncommitted, because it's (a) currently always 0 on Linux and Mac, and (b) on Windows that space costs us almost nothing (just address space) and so it's inherently uninteresting.  "explicit" isn't guaranteed to include absolutely everything anyway so I think this is the right trade-off.

Comment 12

[Luke Wagner [:luke]]

Comment on attachment 536826 [details] [diff] [review]
patch

r+ sans uncommittedSize() and GetJSStackSpaceUncommitted, per previous comment.

Comment 13

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/bf418b38bfe3

Comment 14

[Chris Leary [:cdleary] (not checking bugmail)]

hg.mozilla.org/mozilla-central/rev/bf418b38bfe3