Closed
Bug 741378
Opened 12 years ago
Closed 12 years ago
Rejigger JS memory reporters to mirror jemalloc's
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: glandium, Assigned: justin.lebar+bug)
References
Details
(Whiteboard: [MemShrink:P2])
Attachments
(2 files)
10.39 KB,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
3.19 KB,
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
Right after a fresh start of a local build with jemalloc, here is what I can see in about:memory: 77,393,920 B (100.0%) -- resident ├──42,512,384 B (54.93%) -- anonymous │ ├──42,168,320 B (54.49%) ── anonymous, outside brk() [rw-p] [42] │ └─────344,064 B (00.44%) ── anonymous, outside brk() [rwxp] [5] ├──33,800,192 B (43.67%) ++ shared-libraries ├─────995,328 B (01.29%) ++ other-files ├──────81,920 B (00.11%) ── main thread's stack [rw-p] └───────4,096 B (00.01%) ── vdso [r-xp] 29,451,480 B ── heap-allocated 34,652,160 B ── heap-committed 14.96% ── heap-committed-fragmentation 2,428,928 B ── heap-dirty 14,586,128 B ── heap-unallocated I would expect heap-committed to somehow match anonymous RSS. On an actual (crazy) session (crazy meaning that it has about 2000 tabs, although only a few are loaded), it looks like this: 1,568,550,912 B (100.0%) -- resident ├──1,535,102,976 B (97.87%) -- anonymous │ ├──1,528,332,288 B (97.44%) ── anonymous, outside brk() [rw-p] [177] │ └──────6,770,688 B (00.43%) ── anonymous, outside brk() [rwxp] [46] ├─────31,657,984 B (02.02%) ++ shared-libraries ├──────1,572,864 B (00.10%) ++ other-files ├────────212,992 B (00.01%) ── main thread's stack [rw-p] └──────────4,096 B (00.00%) ── vdso [r-xp] 1,043,472,504 B ── heap-allocated 1,079,672,832 B ── heap-committed 3.35% ── heap-committed-fragmentation 1,523,712 B ── heap-dirty 64,871,168 B ── heap-unallocated The difference would seem to come from the gc-heap, for which we don't have a committed value.
Assignee | ||
Comment 1•12 years ago
|
||
Could the gc-heap be 500mb, or is it obviously smaller? Most of the GC heap should be committed, except for js-main-runtime-gc-heap-decommitted.
Reporter | ||
Comment 2•12 years ago
|
||
It's 673M on the latter case, and 10M on the former case, which makes a significant amount of it not committed.
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #2) > It's 673M on the latter case, and 10M on the former case, which makes a > significant amount of it not committed. Yes, I think JS's accounting assumes that as soon as it mmap's a chunk, that whole chunk is committed. If it later decommits part of the chunk, that is tracked in gc-heap-decommitted. It seems likely to me that the bulk of the difference between anonymous RSS and heap-committed is due to the JS engine. Do you agree? If so, what would you like to get fixed here, exactly? JS's accounting of what's committed?
Reporter | ||
Comment 4•12 years ago
|
||
Let's see: heap-committed + js-gc-heap - jsmain-runtime-gc-heap-decommitted = 1,079,672,832 + 673,185,792 - 198,139,904 = 1,554,718,720 Compared to rss anonymous = 1,535,102,976 I'd say close enough (probably). It might be helpful to have these things made clearer in about:memory. Having heap-committed instead of heap-decommitted for gc-heap could be a step forward. Additionally, it may be helpful to have something like heap-allocated for gc-heap, as well as a fragmentation indicator.
Reporter | ||
Comment 5•12 years ago
|
||
Also, something like js-main-runtime-gc-heap-unused-fraction that does not include js-main-runtime-gc-decommitted might be helpful. Because 44.72% is pretty scary, but 21% is less scary (which is what i get for (js-main-runtime-gc-heap-arena-unused + js-main-runtime-gc-heap-chunk-clean-unused + js-main-runtime-gc-heap-chunk-dirty-unused) / (js-gc-heap - js-main-runtime-gc-heap-decommitted) [which is still a lot by the way]
Assignee | ||
Comment 6•12 years ago
|
||
> Yes, I think JS's accounting assumes that as soon as it mmap's a chunk, that whole chunk is > committed. If it later decommits part of the chunk, that is tracked in gc-heap-decommitted. Okay, so maybe this was wrong! That would be nice. > Having heap-committed instead of heap-decommitted for gc-heap could be a step forward. I could get behind this. > Additionally, it may be helpful to have something like heap-allocated for gc-heap FWIW I believe that's currently gc-heap-committed - gc-heap-arena-unused. But we could rejigger these, sure. > Also, something like js-main-runtime-gc-heap-unused-fraction that does not include js-main- > runtime-gc-decommitted might be helpful. Sure, agreed. > 21% is still a lot by the way Close your eyes, click your heels, and say "there's no place like a compacting generational garbage collector"...
Assignee | ||
Comment 7•12 years ago
|
||
Nick, what do you think about the changes above?
Assignee | ||
Updated•12 years ago
|
Summary: Unexpected big difference between heap-committed and anonymous rss → Rejigger JS memory reporters to mirror jemalloc's
Comment 8•12 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #6) > > Yes, I think JS's accounting assumes that as soon as it mmap's a chunk, that whole chunk is > > committed. If it later decommits part of the chunk, that is tracked in gc-heap-decommitted. > > Okay, so maybe this was wrong! That would be nice. You were right originally, actually. The idea is that since we are out of memory and allocating more, we are probably going to want to use that memory pretty soon. If we have not used it by the next GC, then we decommit it at that point. The hope is that doing it this way we will take fewer page faults on average. I have no solid data supporting this -- it's probably not a terribly important effect either way. As you point out, the real right solution is a compacting GC, and we're working hard to make that happen. > > 21% is still a lot by the way > > Close your eyes, click your heels, and say "there's no place like a > compacting generational garbage collector"... Coming soon to a SpiderMonkey near you.
Assignee | ||
Comment 9•12 years ago
|
||
> If we have not used it by the next GC, then we decommit it at that point.
Not every GC will end up decommitting, right? It has to be a "big" one, for some definition of "big". How often do those run?
(The heuristic may be fine, but we may still want to adjust the memory reporter to reflect the reality that memory isn't actually committed until it's touched.)
Comment 10•12 years ago
|
||
(In reply to Justin Lebar [:jlebar] from comment #9) > > If we have not used it by the next GC, then we decommit it at that point. > > Not every GC will end up decommitting, right? It has to be a "big" one, for > some definition of "big". How often do those run? Someone more knowledgeable will have to chime in to answer that: our GC scheduling is bafflingly complex (and probably not complex enough yet). > (The heuristic may be fine, but we may still want to adjust the memory > reporter to reflect the reality that memory isn't actually committed until > it's touched.) I'm all for making about:memory more user friendly: it's not just a debug tool anymore. That said, I also don't think it makes much sense to quibble over an extra 1MiB that goes away on GC, particularly when it's just one of many other caches. Perhaps it would make sense to automatically run a full GC when we browse to about:memory? Not having to worry about caches or page states would simplify the problem quite a bit.
Assignee | ||
Comment 11•12 years ago
|
||
> Perhaps it would make sense to automatically run a full GC when we browse to about:memory? Not having > to worry about caches or page states would simplify the problem quite a bit. We do this in about:compartments, but we've resisted doing it in about:memory, since you occasionally want to observe the state of the world as it exists, before you trigger that GC. > That said, I also don't think it makes much sense to quibble over an extra 1MiB that goes away on > GC, particularly when it's just one of many other caches. I guess it's at most 1MB, isn't it? In that case, I don't care much at all. :)
Comment 12•12 years ago
|
||
P2 per Memshrink
Assignee: nobody → justin.lebar+bug
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee | ||
Comment 13•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #615207 -
Flags: review?(n.nethercote)
Comment 14•12 years ago
|
||
Comment on attachment 615207 [details] [diff] [review] Patch v1 Review of attachment 615207 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCJSRuntime.cpp @@ +1776,5 @@ > + REPORT_BYTES(NS_LITERAL_CSTRING("js-main-runtime-gc-heap-unused"), > + nsIMemoryReporter::KIND_OTHER, > + rtStats.gcHeapUnused, > + "Amount of the GC heap that's committed, but that is " > + "neither part of an active allocation and nor being for " "and nor being for" -> "nor being used for" @@ +1777,5 @@ > + nsIMemoryReporter::KIND_OTHER, > + rtStats.gcHeapUnused, > + "Amount of the GC heap that's committed, but that is " > + "neither part of an active allocation and nor being for " > + "bookkeeping. Equal to 'gc-heap-chunk-dirty-unused' + " Elsewhere we use "book-keeping". Looks like "bookkeeping" is preferred -- can you remove hypens in the existing cases? @@ +1801,2 @@ > "Fraction of the main JSRuntime's garbage-collected JavaScript " > "heap that is unused. Computed as " Maybe say "fraction of the committed part of the main JSRuntime's heap"?
Attachment #615207 -
Flags: review?(n.nethercote) → review+
Assignee | ||
Comment 15•12 years ago
|
||
Mind if I rename gc-heap-unused --> gc-heap-committed-unused?
Assignee | ||
Comment 16•12 years ago
|
||
Er, nevermind, that doesn't work with the other -unused reporters we have. :(
Assignee | ||
Comment 17•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/21f34fdd64a3
Component: jemalloc → JavaScript Engine
QA Contact: jemalloc → general
Assignee | ||
Comment 18•12 years ago
|
||
... and backed out (before any tests even started) because telemetry uses js-gc-heap, which I removed.
Assignee | ||
Comment 19•12 years ago
|
||
Bill, Terrence, would you mind if instead of reporting |js-gc-heap|, we reported js-gc-heap-committed in Telemetry?
Assignee | ||
Comment 20•12 years ago
|
||
Attachment #615220 -
Flags: review?(wmccloskey)
Assignee | ||
Updated•12 years ago
|
Attachment #615220 -
Attachment description: Part 2: Send js-gc-heap-committed instead of js-gc-heap. → Part 2: Send js-gc-heap-committed instead of js-gc-heap in telemetry.
Comment on attachment 615220 [details] [diff] [review] Part 2: Send js-gc-heap-committed instead of js-gc-heap in telemetry. I'm not aware of anyone ever using this data, so it seems fine to change it.
Attachment #615220 -
Flags: review?(wmccloskey) → review+
Comment 22•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/48315737bd3f https://hg.mozilla.org/mozilla-central/rev/29c2bb6184b6 https://hg.mozilla.org/mozilla-central/rev/1c46346f8e68
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Assignee | ||
Comment 23•12 years ago
|
||
Alas, this still broke js-gc-heap-committed telemetry, because that value is contained in a multi-reporter, which isn't supported in telemetry ping. We should back this out of Aurora so we don't have a big gap in our telemetry. The fix for nightly is in bug 748440.
Assignee | ||
Updated•12 years ago
|
status-firefox14:
--- → affected
status-firefox15:
--- → affected
tracking-firefox14:
--- → ?
tracking-firefox15:
--- → ?
Keywords: regression
Assignee | ||
Updated•12 years ago
|
status-firefox14:
affected → ---
status-firefox15:
affected → ---
tracking-firefox14:
? → ---
tracking-firefox15:
? → ---
Keywords: regression
You need to log in
before you can comment on or make changes to this bug.
Description
•