559326 - Try to add Cycle collector support to AeroPeek backend

Status: NEW

(Core :: Widget: Win32, defect)

Description

[Marco Bonardo [:mak]]

I already tried without great success, we should check again till we figure out with full confidence if the cycle in bug 545228 can be handled in the cpp part too.

Comment 1

[Robert Sayre]

did you try with Cycle Collector instrumentation turned on?

Comment 2

[Marco Bonardo [:mak]]

not sure what you mean by "instrumentation" (sorry my english fails here), i suppose you talking about the perl scripts in http://mxr.mozilla.org/mozilla/source/tools/rb/? I did not try them, i implemented it directly and checked using the usual output from XPCOM_MEM_LEAK_LOG.

Comment 3

[Robert Sayre]

I meant defining DEBUG_CC in addition to XPCOM_MEM_LEAK_LOG. That way you can see more data on missing edges.

Comment 4

[Jim Mathies [:jimm]]

I've noticed some strange behavior related to tab previews and I'm wondering if this bug would help address it.

STR:
1) open up the dhtml bubble mark demo - 
http://bubblemark.com/dhtml.htm
2) open up task manager and highlight firefox.exe
3) hover over the firefox taskbar icon so that you get a preview rendering of the bubble mark tab.

Typical behavior is one of two cases:

1) cpu spikes
2) memory use climbs upward of around 150 megs, then fall back down, then climbs again, over and over. 

or 

1) cpu spikes
2) memory use climbs upwards around 500 megs, previews die, and sometimes the browser locks or UI/window frame rendering breaks.  

You can make things worse and get to the second case by opening up multiple tabs to the bubble mark dhtml page.

It doesn't seem like we're actually leaking, but something isn't getting garbage collected quickly enough. Maybe we need to do some work on the preview code so that it better manages it's resources?

Comment 5

[Jim Mathies [:jimm]]

Filed Bug 559613

Comment 6

[Rob Arnold [:robarnold]]

It might be worth looking into converting everything into weak pointers. I'm not sure what the performance implications are vs cycle collector but I think it'll be less pain.

TaskbarTabPreview::mNext can just be converted into a weak ptr and there are not many checks - they should all be contained in the move logic. 

TaskbarPreview::mController is a bit trickier. I think we want a helper method to avoid writing the weakptr lookup code everywhere. This helper method could also return a dumb default global controller (we have one already) if the actual controller has been GC'd.

I don't expect cycles with mIconImage or mTaskbar as those have a single native code implementation.