If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Report system allocator memory in about:memory on Linux

RESOLVED FIXED in Firefox 43

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: glandium, Assigned: njn)

Tracking

Trunk
mozilla43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
This is especially important on Android, where parts of libmozglue itself use the system allocator, and all system libraries use the system allocator. I think it would be worthwhile to have on other systems too, and could lead to unpleasant surprises.

I /think/ it should just grow the overall "explicit" size, and "heap-unclassified" as a side effect. We'd also add the raw number somewhere under "other/". I don't think we want to have that entry under "explicit/" because there may well be memory reporters accounting some system allocated memory (and it happens I'm working on one for Android).
(Reporter)

Updated

5 years ago
Blocks: 828845
This sounds good to me.
(Assignee)

Comment 2

5 years ago
What is the system allocator?  Is it a heap allocator (i.e. malloc/free style) that's used by system libs instead of jemalloc?

W.r.t. heap-unclassified, you'll need to be careful.  heap-unclassified is computed like this:

  heap-unclassified = heap-allocated - (sum of all explicit KIND_HEAP reports)

If there are effectively two heaps -- one system and one jemalloc -- then we need to think about what "heap" means in these reporter names.  E.g.:

- Is heap-allocated the sum of the two heaps?
- Or do we distinguish jemalloc-heap-{allocated,unclassified} from system-heap-{allocated,unclassified}?

Also, does DMD see the system heap?
Whiteboard: [MemShrink]
(Reporter)

Comment 3

5 years ago
(In reply to Nicholas Nethercote [:njn] from comment #2)
> What is the system allocator?  Is it a heap allocator (i.e. malloc/free
> style) that's used by system libs instead of jemalloc?

Yes. On android, everything outside gecko uses the system malloc, and a small part of libmozglue does as well. On Linux, nothing should be using the system malloc. On Mac, I suspect a few system libraries use other zones, and a few allocations may happen before we hook jemalloc in. On Windows, I think things are using the system CRT malloc/operator new, and we are totally blind about them.

> W.r.t. heap-unclassified, you'll need to be careful.  heap-unclassified is
> computed like this:
> 
>   heap-unclassified = heap-allocated - (sum of all explicit KIND_HEAP
> reports)
> 
> If there are effectively two heaps -- one system and one jemalloc -- then we
> need to think about what "heap" means in these reporter names.  E.g.:
> 
> - Is heap-allocated the sum of the two heaps?
> - Or do we distinguish jemalloc-heap-{allocated,unclassified} from
> system-heap-{allocated,unclassified}?

I think under "other measurements", we'd have a distinction between jemalloc and system heap-allocated. But I think we won't be able to make heap-unclassified distinct reliably. But maybe I'm being pessimistic, I don't know.

> Also, does DMD see the system heap?

No.
(Assignee)

Comment 4

5 years ago
Here's the simplest possibility I can think of:

- Measure "system-heap-allocated".  It would go in "Other Measurements".

- In about:memory, change the "heap-unclassified" measurement to this:

  heap-unclassified = (heap-allocated + system-heap-allocated) -
                      (sum of all explicit KIND_HEAP reports)

The sad thing about this is that because DMD cannot see into the system heap, there would be an unknown fraction of heap-unclassified that we'd never have insight into.


A more complex, but possibly better possibility:

- Measure "system-heap-allocated".  It would go in "Other Measurements".

- In nsIMemoryReporter, add a new KIND_SYSTEM_HEAP kind.

- In about:memory, leave the "heap-unclassified" measurement unchanged.

- In about:memory, add a "system-heap-unclassified" measurement:

  system-heap-unclassified = system-heap-allocated -
                             (sum of all explicit KIND_SYSTEM_HEAP reports)

This would preserve the "DMD tells you all about heap-unclassified" property.  We wouldn't have any insight into system-heap-unclassified, of course.

This also requires us to be able to accurately determine within every memory reporter whether each reported heap block is KIND_HEAP or KIND_SYSTEM_HEAP.  I don't know if that's possible.  (I suspect that we might be able to do this, because I doubt the reporters for system memory will be using moz_malloc_size_of...)
(Assignee)

Updated

5 years ago
Whiteboard: [MemShrink] → [MemShrink:P2]
(Reporter)

Updated

5 years ago
No longer blocks: 828845
(Assignee)

Comment 5

2 years ago
FWIW, here's what I did in Servo, where the system allocator is used a *lot* (for all the non-Rust libraries, basically):

- Memory reporters distinguish between jemalloc allocations and system allocations.

- "explicit" covers both jemalloc allocations and system allocations.

- "jemalloc-heap-unclassified" and "system-heap-unclassified" both occur under "explicit".

- "jemalloc-heap-allocated" and "system-heap-allocated" both go in the non-"explicit" measurements.

This is very similar to the "more complex" proposal from comment 4.
(Assignee)

Comment 6

2 years ago
As for how to get the size of the system heap:

- Linux: call mallinfo(), and return |info.hblkhd + info.uordblks|

- Android: any difference with Linux?

- Mac: I don't know.

- Windows: I don't know. Well, you can enumerate a heap and measure each block individually, but that sounds slow and awful: https://msdn.microsoft.com/en-us/library/windows/desktop/ee175819%28v=vs.85%29.aspx
> - Windows: I don't know. Well, you can enumerate a heap and measure each
> block individually, but that sounds slow and awful:

Since we already walk the address space, as an estimate you could take the total number of heap-like bytes (private, committed, rw, maybe some other flags) and subtract the jemalloc size.
(Assignee)

Comment 8

2 years ago
Created attachment 8647143 [details] [diff] [review]
Add a "system-heap-allocated" memory report on Linux

It'd obviously be nice to get this implemented for other platforms, but here's
a start.
Attachment #8647143 - Flags: review?(mh+mozilla)
(Assignee)

Updated

2 years ago
Assignee: mh+mozilla → n.nethercote
Status: NEW → ASSIGNED
(Reporter)

Comment 9

2 years ago
Comment on attachment 8647143 [details] [diff] [review]
Add a "system-heap-allocated" memory report on Linux

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

::: xpcom/base/nsMemoryReporterManager.cpp
@@ +190,5 @@
> +    // The fields in |struct mallinfo| are all |int|, <sigh>, so it is
> +    // unreliable if memory usage gets high. However, the system heap size on
> +    // Linux should usually be zero (so long as jemalloc is enabled) so that
> +    // shouldn't be a problem.
> +    return info.hblkhd + info.uordblks;

You should cast to size_t each of those, so that you add two size_ts instead of adding two ints.

@@ +839,5 @@
> +    // The documentation in the glibc man page makes it sound like |uordblks|
> +    // would suffice, but that only gets the small allocations that are put in
> +    // the brk heap. We need |hblkhd| as well to get the larger allocations
> +    // that are mmapped.
> +    int64_t amount = info.hblkhd + info.uordblks;

You want to use the SystemHeapSize function here.
Attachment #8647143 - Flags: review?(mh+mozilla) → review-
(Assignee)

Comment 10

2 years ago
Created attachment 8647262 [details] [diff] [review]
Add a "system-heap-allocated" memory report on Linux
Attachment #8647262 - Flags: review?(mh+mozilla)
(Assignee)

Updated

2 years ago
Attachment #8647143 - Attachment is obsolete: true
(Reporter)

Comment 11

2 years ago
Comment on attachment 8647262 [details] [diff] [review]
Add a "system-heap-allocated" memory report on Linux

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

::: xpcom/base/nsMemoryReporterManager.cpp
@@ +186,5 @@
> +    // would suffice, but that only gets the small allocations that are put in
> +    // the brk heap. We need |hblkhd| as well to get the larger allocations
> +    // that are mmapped.
> +    //
> +    // The fields in |struct mallinfo| are all |int|, <sigh>, so it is

For the record, to put some perspective, since I researched this briefly, mallinfo first appeared in System V r2 and seems to have been added to the System V Interface Definition issue 2 released in 1986 (although it might have been in issue 1 released in 1985, but I couldn't find a copy online. The computer history museum has a hard copy). It was later deprecated in SVID issue 4 from 1995, noting it would be removed from the next issue that never happened.

20 years later, the function is still available and still has these shortcomings inherited from times where memory was counted in kilobytes.
Attachment #8647262 - Flags: review?(mh+mozilla) → review+

Comment 12

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a920bf34b576
(Assignee)

Comment 13

2 years ago
Since I hijacked this bug to just be about Linux, I filed bug 1194061 as a follow-up for the remaining platforms of interest.
Summary: Report system allocator memory in about:memory → Report system allocator memory in about:memory on Linux
https://hg.mozilla.org/mozilla-central/rev/a920bf34b576
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.