692226 - add weakmap support to cycle collector graph logging

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

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.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: not pretty, but it gets the job done

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.

Comment 2

[Andrew McCreight [:mccr8]]

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.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: fix it the right way

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?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 781107 [details] [diff] [review]
fix it the right way

I

Comment 5

[Andrew McCreight [:mccr8]]

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

Comment 6

[Andrew McCreight [:mccr8]]

Comment on attachment 579816 [details] [diff] [review]
not pretty, but it gets the job done

https://hg.mozilla.org/integration/mozilla-inbound/rev/155ab662755e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/155ab662755e