Closed Bug 546477 Opened 10 years ago Closed 9 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: luke, Assigned: njn)

References

(Blocks 1 open bug)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

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.
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?
(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.
Blocks: DarkMatter
Luke, are the VirtualAlloc calls in question the ones for code generated by the JITs?  If so, bug 633653 has this covered.
The VirtualAlloc calls I was referring to are in StackSpace::init and StackSpace::bumpCommit.
> 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.
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?
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.
See also bug 656941, which proposed to extend the current Windows-only behaviour to all platforms.
Attached patch patchSplinter Review
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.
Assignee: general → nnethercote
Attachment #536826 - Flags: review?(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.)
(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 on attachment 536826 [details] [diff] [review]
patch

r+ sans uncommittedSize() and GetJSStackSpaceUncommitted, per previous comment.
Attachment #536826 - Flags: review?(luke) → review+
http://hg.mozilla.org/tracemonkey/rev/bf418b38bfe3
Whiteboard: fixed-in-tracemonkey
Summary: report JS engine VirtualAlloc usage to about:memory → report JS engine VirtualAlloc usage (for the stack) to about:memory
hg.mozilla.org/mozilla-central/rev/bf418b38bfe3
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.