Closed Bug 713778 Opened 9 years ago Closed 9 years ago

nsICycleCollectorListener should not enable trace_all by default

Categories

(Core :: XPCOM, defect)

10 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: smaug, Assigned: smaug)

Details

Attachments

(1 file)

We optimize many edges out, but nsICycleCollectorListener doesn't know about it
Assignee: nobody → bugs
Attached patch patchSplinter Review
This changes the default to be more useful, IMO, but the old
WANT_ALL_TRACES is still possible if one
uses
window.QueryInterface(Components.interfaces.nsIInterfaceRequestor).
  getInterface(Components.interfaces.nsIDOMWindowUtils).
  garbageCollect(Components.classes["@mozilla.org/cycle-collector-logger;1"]
    .createInstance(Components.interfaces.nsICycleCollectorListener).allTraces())

So, it is enough to just call allTraces(), and since the method returns the listener itself, 
no need to store the listener in any temporary variable.
Attachment #585081 - Flags: review?(continuation)
Comment on attachment 585081 [details] [diff] [review]
patch

Review of attachment 585081 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.  I think this will be a more useful default.  WANT_ALL_TRACES is good for leak hunting, but not for CC performance analysis.

I assume the existing JS incantation will continue to work?  If not, you should update the wiki entry at https://wiki.mozilla.org/Performance:Leak_Tools#Cycle_collector_heap_dump .  Either way, you should probably add this new way to invoke it there.
Attachment #585081 - Flags: review?(continuation) → review+
https://hg.mozilla.org/mozilla-central/rev/d98fbf3cbd71

I updated also the wiki page.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.