Closed Bug 663920 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: taras.mozilla, Assigned: mccr8)

References

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 file, 3 obsolete files)

Attached patch telemetry (obsolete) — Splinter Review
Andrew added a few extra stats to diagnose cycle collector issues.
Attachment #538973 - Flags: review?
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)
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.
Whiteboard: [MemShrink:P2]
Assignee: nobody → continuation
OS: Linux → All
Hardware: x86 → All
Attachment #538973 - Attachment is patch: true
Attached patch telemetry (obsolete) — Splinter Review
Sorry, forgot to qref the previous patch
Attachment #538973 - Attachment is obsolete: true
Attachment #538973 - Flags: review?(peterv)
Attachment #539363 - Flags: review?(peterv)
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)
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+
Addressed review comments.  Carrying forward bent's r+.
Attachment #543470 - Attachment is obsolete: true
Attachment #543480 - Flags: review+
Summary: More cycle collector telemetry → Cycle collector telemetry: num of RCed and GCed nodes in graph, num of objects collected
Blocks: 638299
Whiteboard: [MemShrink:P2] → [MemShrink:P2][inbound]
http://hg.mozilla.org/mozilla-central/rev/f89d4b284590
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Whiteboard: [MemShrink:P2][inbound] → [MemShrink:P2]
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.