684592 - Update heap-committed's description to reflect that heap-committed == heap-allocated + heap-unallocated only on Linux, not on Windows, and update heap-dirty description

Status: RESOLVED FIXED

(Toolkit :: about:memory, defect)

Description

[Justin Lebar (not reading bugmail)]

I asserted that heap-committed == heap-allocated + heap-unallocated in bug 675216, and the description for heap-committed contains this assertion.  But it's demonstrably wrong; just load up a Windows build.

My mistake was reading #ifndef as #ifdef in

#ifndef MALLOC_DECOMMIT
  stats->committed = stats->mapped
#endif

I just checked, and indeed on Linux, heap-committed == heap-allocated + heap-unallocated.

Comment 1

[Justin Lebar (not reading bugmail)]

Might as well fix heap-dirty while I'm at it.

Comment 2

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

Comment 3

[Nicholas Nethercote [inactive]]

Comment on attachment 558185 [details] [diff] [review]
Patch v1

Review of attachment 558185 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me, you know much more about this stuff at this point than I do.

::: xpcom/base/nsMemoryReporterManager.cpp
@@ +441,5 @@
> +#if defined(HAVE_JEMALLOC_STATS) && defined(XP_WIN)
> +    // heap-committed is only meaningful where we have MALLOC_DECOMMIT defined
> +    // (currently, just on Windows).  Elsewhere, it's the same as
> +    // stats->mapped, which is heap-allocated + heap-unallocated.
> +    REGISTER(HeapCommitted);

In that case, shouldn't the condition be this:

#if defined(HAVE_JEMALLOC_STATS) && defined(MALLOC_DECOMMIT)

?  Or is MALLOC_DECOMMIT not visible here?

Comment 4

[Justin Lebar (not reading bugmail)]

> Or is MALLOC_DECOMMIT not visible here?

Yeah, it's #define'd in jemalloc.c, not in a header.

Comment 5

[Nicholas Nethercote [inactive]]

> Yeah, it's #define'd in jemalloc.c, not in a header.

A comment explaining that would be good!

Comment 6

[Justin Lebar (not reading bugmail)]

inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/f921f632208e

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/f921f632208e