989035 - Account for malloced data hanging off things in the nursery

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Terrence Cole [:terrence]]

Attachment: track_nursery_malloc_data-v0.diff

Appears to be working correctly and still builds with ggc disabled.

Comment 1

[Nicholas Nethercote [inactive]]

Comment on attachment 8398057 [details] [diff] [review]
track_nursery_malloc_data-v0.diff

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

Oh dear. I'm 90% of the way through writing the same patch. Oh well, it makes the review easy.

::: js/public/MemoryMetrics.h
@@ +182,5 @@
>  #define FOR_EACH_SIZE(macro) \
>      macro(_, _, marker) \
>      macro(_, _, nurseryCommitted) \
>      macro(_, _, nurseryDecommitted) \
> +    macro(_, _, nurseryMallocData) \

I called this nurseryHeapSlots.

Comment 2

[Steve Fink [:sfink] [:s:]]

(In reply to Nicholas Nethercote [:njn] from comment #1)
> Comment on attachment 8398057 [details] [diff] [review]
> track_nursery_malloc_data-v0.diff
> 
> Review of attachment 8398057 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Oh dear. I'm 90% of the way through writing the same patch. Oh well, it
> makes the review easy.
> 
> ::: js/public/MemoryMetrics.h
> @@ +182,5 @@
> >  #define FOR_EACH_SIZE(macro) \
> >      macro(_, _, marker) \
> >      macro(_, _, nurseryCommitted) \
> >      macro(_, _, nurseryDecommitted) \
> > +    macro(_, _, nurseryMallocData) \
> 
> I called this nurseryHeapSlots.

I believe it covers both slots and elements, and you can't distinguish which is which.

Comment 3

[Nicholas Nethercote [inactive]]

> I believe it covers both slots and elements, and you can't distinguish which
> is which.

"nurserySlotsAndElements" then. Or "nurseryHugeSlots", to match the member name.

Comment 4

[Terrence Cole [:terrence]]

Attachment: track_nursery_malloc_data-v1.diff

Applied review commentary and added the malloc data from the hashtable itself, which we were missing before.

Comment 5

[Nicholas Nethercote [inactive]]

Comment on attachment 8398247 [details] [diff] [review]
track_nursery_malloc_data-v1.diff

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

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +2357,5 @@
>  
> +    RREPORT_BYTES(rtPath + NS_LITERAL_CSTRING("runtime/gc/nursery-huge-slots"),
> +        KIND_NONHEAP, rtStats.runtime.gc.nurseryHugeSlots,
> +        "Memory being used by the GC's nursery for slots and "
> +        "elements.");

I recently got rid of most of the "Memory used for..." prefixes, because they're redundant. And the out-of-line-ness should be mentioned. So maybe: "Out-of-line slots and elements belonging to objects in the nursery." ?

Comment 6

[Terrence Cole [:terrence]]

Attachment: track_nursery_malloc_data-v2.diff

Thanks, that's much better wording.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/39cf2e4df30d

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/39cf2e4df30d