Closed Bug 692226 Opened 8 years ago Closed 6 years ago

add weakmap support to cycle collector graph logging

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: mccr8, Assigned: mccr8)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [MemShrink])

Attachments

(1 file, 1 obsolete file)

nsICycleCollectorListener should have a way to get real information about weak mappings the cycle collector sees.  Right now, I have a hacked up way to shove it through the interface, but it should be done for real.  This includes extending the implementation to log it, extending the parser library to deal with it.  I should also update the analysis scripts to deal with weak map edges.  Not a huge priority, but it will make debugging things in the future easier.
This will dump out all the WeakMap bindings that the CC sees at the start of the file.  The scripts don't understand this format, but it is useful for debugging.
In B2G leak hunting, we found an iframe that was held alive only due to being a weak map value, so I should really fix this.
Assignee: nobody → continuation
Whiteboard: [MemShrink]
This is a proper fix: it adds a new callback to nsICycleCollectorListener for weak maps.  I still need to update the CC log parser to understand this new kind of entry.

This does not update nsICycleCollectorHandler to report weak map entries.  I have a half-done patch to do that, but I'm not sure it really makes sense.  I think adding a new callback will just break existing CC-analyzing addons, and probably none of them really care about weak map edges right now, so I'm tempted to just leave it alone.  I can file a followup bug with my patch, ready to land in the case anybody cares.  Olli, what do you think?
Flags: needinfo?(bugs)
Attachment #781107 - Flags: review?(bugs)
Comment on attachment 781107 [details] [diff] [review]
fix it the right way

I
Attachment #781107 - Flags: review?(bugs) → review+
Flags: needinfo?(bugs)
I pushed changes to my CC log parser to parse and store these edges, but I'm not really sure the right way to incorporate it into find_roots, so for now, nothing is done with these edges.

I pushed to try to make sure this doesn't break anything weird: https://tbpl.mozilla.org/?tree=Try&rev=4d4e68024d72
https://hg.mozilla.org/mozilla-central/rev/155ab662755e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.