Closed Bug 1125514 Opened 9 years ago Closed 9 years ago

Use metadata statistics to measure bookkeeping

Categories

(Core :: Memory Allocator, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: ggp, Unassigned)

References

Details

Attachments

(1 file)

Metadata statistics have landed upstream [1], and we should now be able to measure bookkeeping as stats.metadata - stats.arena.<i>.metadata.allocated when we next update our clone of jemalloc.

1- https://github.com/jemalloc/jemalloc/commit/4581b97809e7e545c38b996870a4e7284a620bc5
Nice! I'd be interested to see if we get the same numbers between our method and Jason's.
The numbers are different, and Jason's is the correct one.

We compute bookkeeping as <base allocations> + stats.chunks.current * <size of chunk headers> where stats.chunks.current is documented as "Total number of chunks actively mapped on behalf of the application. This does not include inactive chunks." [1]

However, it turns out that chunks allocated for the base allocator are also included in the statistic [2], so their headers are double-counted with our approach. Jason's patch, on the other hand, explicitly acccounts for chunk headers only when they're allocated through arenas [3].

It seems to me that the behavior of stats.chunks.current contradicts the documentation, as these chunks are not mapped "on behalf of the application", but instead serve internal data structures. Of course, being the one who misinterpreted it in the first place, I'm biased :) Compensating for that in our patch does make it equivalent to Jason's, though.

1- http://www.canonware.com/download/jemalloc/jemalloc-latest/doc/jemalloc.html
2- curchunks is always incremented at https://github.com/jemalloc/jemalloc/blob/dev/src/chunk.c#L207, even when |base == true|
3- https://github.com/jemalloc/jemalloc/blob/dev/src/arena.c#L410
This patch is based on https://hg.mozilla.org/try/rev/78b7a32e4ec3, which updates jemalloc to its latest revision. I'd still like to test it a bit more when that revision hits m-c, but it seems to be working well upon a quick test on an optimized build.
Attachment #8579013 - Flags: review?(mh+mozilla)
Attachment #8579013 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8579013 [details] [diff] [review]
Use jemalloc's metadata statistics to compute bookkeeping.

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

::: memory/build/mozjemalloc_compat.c
@@ +125,5 @@
>                  continue;
>              }
>  
> +            CTL_I_GET("stats.arenas.0.metadata.allocated", ameta, i);
> +            stats_ametadata += ameta;

Why iterate over arenas manually instead of using the same approach as pdirty in jemalloc_stats_impl()?

  /* get the summation for all arenas, i == narenas */
  CTL_I_GET("stats.arenas.0.metadata.allocated", stats_ametadata, narenas);
Feel free to file a followup bug with a patch :)
https://hg.mozilla.org/mozilla-central/rev/ffbd5f5f46f1
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Depends on: 1201462
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: