Closed Bug 989035 Opened 6 years ago Closed 6 years ago

Account for malloced data hanging off things in the nursery

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31
Tracking Status
firefox31 --- fixed

People

(Reporter: terrence, Assigned: terrence)

Details

(Whiteboard: [MemShrink][qa-])

Attachments

(1 file, 2 obsolete files)

Appears to be working correctly and still builds with ggc disabled.
Attachment #8398057 - Flags: review?(n.nethercote)
No longer blocks: GenerationalGC
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.
Attachment #8398057 - Flags: review?(n.nethercote) → review+
Whiteboard: [MemShrink]
(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.
> 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.
Applied review commentary and added the malloc data from the hashtable itself, which we were missing before.
Attachment #8398057 - Attachment is obsolete: true
Attachment #8398247 - Flags: review+
Keywords: checkin-needed
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." ?
Thanks, that's much better wording.
Attachment #8398247 - Attachment is obsolete: true
Attachment #8398294 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/39cf2e4df30d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Whiteboard: [MemShrink] → [MemShrink][qa-]
You need to log in before you can comment on or make changes to this bug.