All users were logged out of Bugzilla on October 13th, 2018

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

RESOLVED FIXED in mozilla9

Status

()

RESOLVED FIXED
7 years ago
7 years ago

People

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

Tracking

unspecified
mozilla9
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
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.
(Assignee)

Updated

7 years ago
Depends on: 675216
(Assignee)

Comment 1

7 years ago
Might as well fix heap-dirty while I'm at it.
(Assignee)

Updated

7 years ago
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
(Assignee)

Comment 2

7 years ago
Created attachment 558185 [details] [diff] [review]
Patch v1
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+
(Assignee)

Comment 4

7 years ago
> 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!

Comment 7

7 years ago
http://hg.mozilla.org/mozilla-central/rev/f921f632208e
Assignee: nobody → justin.lebar+bug
Status: NEW → RESOLVED
Last Resolved: 7 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.