Closed Bug 948755 Opened 6 years ago Closed 6 years ago
Log incremental cycle collector roots
With ICC enabled, nodes can be treated as live for reasons other than having unknown references, in ScanIncrementalRoots. We should note these in the cycle collector.
I think this needs to be done before it can be preffed on by default. I've hit at least one case where it was necessary for an investigation. One simple way to do this is to add a fake rooted object to the graph, and add all the incrementally rooted objects as fields of that object. Though I suppose that will mess up the ref counts, so I should do something more correct, like add a new callback to indicate that it is an incremental root.
Assignee: nobody → continuation
I've also updated my scripts to handle incremental roots.
Attachment #8391225 - Flags: review?(bugs)
Comment on attachment 8391225 [details] [diff] [review] Log incremental cycle collector roots. How do you create a log from icc? With env variables only? Since I thought DOMWindowUtils way makes us create non-icc log. Could you add a comment why GraphWalker<ScanBlackVisitor>(ScanBlackVisitor(mWhiteNodeCount, failed)).Walk(pi); handling can be a bit inconsistent. (Like, if we're black, it is no-op anyway) update uuid of nsICycleCollectorListener
Attachment #8391225 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #3) > How do you create a log from icc? With env variables only? > Since I thought DOMWindowUtils way makes us create non-icc log. Yup. If you are interested in analyzing a leak, I don't think you want an ICC log, but it is important for diagnosing ICC issues. > Could you add a comment why > GraphWalker<ScanBlackVisitor>(ScanBlackVisitor(mWhiteNodeCount, > failed)).Walk(pi); handling can be > a bit inconsistent. (Like, if we're black, it is no-op anyway) Yeah, I should explain that whole section, as it is pretty weird. > update uuid of nsICycleCollectorListener Oops, thanks.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.