Closed Bug 684592 Opened 8 years ago Closed 8 years ago

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

Categories

(Toolkit :: about:memory, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: justin.lebar+bug, Assigned: justin.lebar+bug)

References

Details

Attachments

(1 file)

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.
Depends on: 675216
Might as well fix heap-dirty while I'm at it.
Summary: Update heap-committed's description to reflect that heap-committed == heap-allocated + heap-unallocated only on Linux, not on Windows → 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
Attached patch Patch v1Splinter Review
Attachment #558185 - Flags: review?(nnethercote)
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?
Attachment #558185 - Flags: review?(nnethercote) → review+
> Or is MALLOC_DECOMMIT not visible here?

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

A comment explaining that would be good!
http://hg.mozilla.org/mozilla-central/rev/f921f632208e
Assignee: nobody → justin.lebar+bug
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
You need to log in before you can comment on or make changes to this bug.