It would be great to have some tool to analyze possible runtime leaks and mem usage a page can cause. Currently there is just, AFAIK, one implementation of nsICycleCollectorListener, but maybe there could be another one for devtools.

nice idea! What are you thinking for a display? Maybe an event in the web console?

event? I'm not quite sure how CC could be utilized. Would need to brainstorm this a bit :) Perhaps the log could be created using allTraces() https://wiki.mozilla.org/Performance:Leak_Tools#Cycle_collector_heap_dump and then JS could visualize the graph. Basically trying to show what all objects are alive (in CC terms) in a tab.

about:cc

Ugly UI, but it has most of the functionality I use from my perl script.

I tried the extension with Firefox Nightly: Built from http://hg.mozilla.org/mozilla-central/rev/d45c7d7b0079 and I am seeing an exception, whenever I click "Run cycle collector" Timestamp: 16.2.2012 10:05:48 Error: uncaught exception: [Exception... "Cannot modify properties of a WrappedNative" nsresult: "0x80570034 (NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN)" location: "JS frame :: resource://cclog-addon/aboutcc.js :: cc :: line 64" data: no] Clicking "Number of garbage objects" or "Number of root objects" says 0 (but I guess it could be related to that exception) (running on Win Vista) Honza

Honza, the addon requires the core patch (which is waiting for review).

Comment on attachment 597515 [details] [diff] [review] wip Review of attachment 597515 [details] [diff] [review]: ----------------------------------------------------------------- Well, I'm not sure if I'm on board with mashing two different listeners together, if only because of all the getter/setter stuff it requires, but here are some comments on what you have. Thanks a lot for doing this, I've been meaning to look at it, but I haven't gotten around to it. I wonder if it would make sense/is possible to split this off into another file. ::: xpcom/base/nsCycleCollector.cpp @@ +1424,5 @@ > + > + enum Type > + { > + eRefCountedObject, > + eGCedObject, you could have a type for marked and unmarked GCed objects, then avoid the bool mGCMarked. Not a big deal. @@ +1444,5 @@ > { > public: > + nsCycleCollectorLogger() : > + mStream(nsnull), mWantAllTraces(false), > + mDisableLog(false), mWantAfterProcessing(false), These two flags should be consistent, so you don't check one for true and one for false all over the place. Maybe mWantLogToFile and mWantSaveLog or something? @@ +1649,5 @@ > > + NS_IMETHOD ProcessNext(nsICycleCollectorHandler* aHandler, > + bool* aCanContinue) > + { > + NS_ENSURE_STATE(aHandler); Do we maybe want an assert here for mWantAfterProcessing?

> > + enum Type > > + { > > + eRefCountedObject, > > + eGCedObject, > > you could have a type for marked and unmarked GCed objects, then avoid the > bool mGCMarked. Not a big deal. Ah, yes. > > @@ +1444,5 @@ > > { > > public: > > + nsCycleCollectorLogger() : > > + mStream(nsnull), mWantAllTraces(false), > > + mDisableLog(false), mWantAfterProcessing(false), > > These two flags should be consistent, so you don't check one for true and > one for false all over the place. Maybe mWantLogToFile and mWantSaveLog or > something? Well, I wanted to be consistent with the IDL attribute names > > @@ +1649,5 @@ > > > > + NS_IMETHOD ProcessNext(nsICycleCollectorHandler* aHandler, > > + bool* aCanContinue) > > + { > > + NS_ENSURE_STATE(aHandler); > > Do we maybe want an assert here for mWantAfterProcessing? No. It is perfectly legal for JS to call the method whenever it wants. But, perhaps the method should throw if !mWantAfterProcessing I could change to NS_ENSURE_STATE(aHandler && mWantAfterProcessing);

(In reply to Olli Pettay [:smaug] from comment #12) > > Do we maybe want an assert here for mWantAfterProcessing? > No. It is perfectly legal for JS to call the method whenever it wants. > But, perhaps the method should throw if !mWantAfterProcessing > I could change to NS_ENSURE_STATE(aHandler && mWantAfterProcessing); Ah, good point it will be JS calling it. I think it is safe to run it without that, but people may want to know more strongly that they aren't going to get anything. Though I guess the "JS way" is to just keep plowing ahead and ignore errors as long as you can, so maybe it is fine as is...

Well, throwing (NS_ENSURE_*) gives a good hint that something is wrong :)

(In reply to Olli Pettay [:smaug] from comment #12) > Well, I wanted to be consistent with the IDL attribute names Sure, but you could change the IDL attribute name, too. Not a big deal, I guess.

Comment on attachment 597515 [details] [diff] [review] wip Review of attachment 597515 [details] [diff] [review]: ----------------------------------------------------------------- I still think it is a little gross to mash together two listeners like this, but it really isn't a huge deal, so okay.

Great progress here, Olli! I created another extension that provides more interactive UI (the graph is an expandable tree). It doesn't have all the logic yet, but already shows the object graph, roots and documents. The source is available here: https://github.com/janodvarko/ccdump Honza

Does(In reply to Jan Honza Odvarko from comment #21) > Created attachment 598896 [details] Does this end up creating/deleting dom nodes? For an addon like about:cc it is pretty important to not create any extra cycle collectable garbage. That would reduce the reliability.

Though, I guess one could make things more reliable if after deleting some parts of a dom tree you run cycle collector few times.

Honza, I get the following when loading about:ccdump. Timestamp: 02/20/2012 07:49:15 PM Error: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIXPCComponents_Utils.import]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: resource://ccdump/content/lib/trace.js :: <TOP_LEVEL> :: line 17" data: no] Source File: resource://ccdump/content/lib/trace.js Line: 22

One bug fixed in this version Honza

Comment on attachment 598919 [details] CCDump 3 (yet one problem fixed) Marking this obsolete since Honza has better version in https://github.com/janodvarko/ccdump

nsICycleCollectorListener.idl needs comments about the new interface and methods.

Comment on attachment 598349 [details] cc analyzer The latest version should work with about:telemetry