Cycle collector telemetry: num of RCed and GCed nodes in graph, num of objects collected

RESOLVED FIXED in mozilla7

Status

()

Core
XPCOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: (dormant account), Assigned: mccr8)

Tracking

unspecified
mozilla7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 538973 [details] [diff] [review]
telemetry

Andrew added a few extra stats to diagnose cycle collector issues.
Attachment #538973 - Flags: review?
(Reporter)

Comment 1

6 years ago
Comment on attachment 538973 [details] [diff] [review]
telemetry

This is Andrew's patch, I just updated the telemetry probes
Attachment #538973 - Flags: review? → review?(peterv)
(Assignee)

Comment 2

6 years ago
Another thing we could think about tracking is the number of edges.  This could be done fairly efficiently (if approximately) by tracking the number of blocks allocated for edges.  However, the number of nodes is probably a decent proxy for the number of edges.
(Assignee)

Updated

6 years ago
Blocks: 640791
Whiteboard: [MemShrink:P2]
(Assignee)

Updated

6 years ago
Assignee: nobody → continuation
OS: Linux → All
Hardware: x86 → All
Attachment #538973 - Attachment is patch: true
(Reporter)

Comment 3

6 years ago
Created attachment 539363 [details] [diff] [review]
telemetry

Sorry, forgot to qref the previous patch
Attachment #538973 - Attachment is obsolete: true
Attachment #538973 - Flags: review?(peterv)
Attachment #539363 - Flags: review?(peterv)
(Assignee)

Comment 4

6 years ago
Created attachment 543470 [details] [diff] [review]
add telemetry for the num of RCed and GCed nodes visited by the CC, and how many were collected

Unbitrotted the previous patch, and did a little bit of cleanup to make the naming consistent.  I also added telemetry for the number of nodes we collected, which was easy because the CC already tracks that number.
Attachment #539363 - Attachment is obsolete: true
Attachment #539363 - Flags: review?(peterv)
Attachment #543470 - Flags: review?(bent.mozilla)
Attachment #543470 - Flags: feedback?(tglek)
(Reporter)

Updated

6 years ago
Attachment #543470 - Flags: feedback?(tglek) → feedback+
Comment on attachment 543470 [details] [diff] [review]
add telemetry for the num of RCed and GCed nodes visited by the CC, and how many were collected

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

::: xpcom/base/nsCycleCollector.cpp
@@ +1021,5 @@
>  
>      nsPurpleBuffer mPurpleBuf;
>  
> +    PRUint32 mVisitedRefCounted;
> +    PRUint32 mVisitedGCed;

Nit: Comment here that these are only used for telemetry tracking.

@@ +2205,5 @@
>      mPtrLog(nsnull)
>  #else
> +    mPurpleBuf(mParams),
> +    mVisitedRefCounted(0),
> +    mVisitedGCed(0)

Hm, you're not initializing these in the non-DEBUG_CC case. I'd move them before mPurpleBuf in the class decl as well as here so that you don't get affected by the #ifdef.
Attachment #543470 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 6

6 years ago
Created attachment 543480 [details] [diff] [review]
add telemetry for the num of RCed and GCed nodes visited by the CC, and how many were collected

Addressed review comments.  Carrying forward bent's r+.
Attachment #543470 - Attachment is obsolete: true
Attachment #543480 - Flags: review+
(Assignee)

Updated

6 years ago
Summary: More cycle collector telemetry → Cycle collector telemetry: num of RCed and GCed nodes in graph, num of objects collected
(Assignee)

Updated

6 years ago
Blocks: 638299
Whiteboard: [MemShrink:P2] → [MemShrink:P2][inbound]
http://hg.mozilla.org/mozilla-central/rev/f89d4b284590
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
(Assignee)

Updated

6 years ago
Whiteboard: [MemShrink:P2][inbound] → [MemShrink:P2]
(Assignee)

Comment 8

6 years ago
I've been keeping an eye on this on my own computer since it landed.  It mostly looks reasonable, but the buckets for the number of objects being collected will probably need to be adjusted.  There are buckets for 0, 2, 4, 8, 10, 13, 16, etc. with things in them that could probably be combined together more.  In addition, I already have one collection with 100k objects in it, which probably maxed it out.
You need to log in before you can comment on or make changes to this bug.