Last Comment Bug 673252 - Add MOZ_QUIET env var to suppress ++DOCSHELL, ++DOMWINDOW printfs
: Add MOZ_QUIET env var to suppress ++DOCSHELL, ++DOMWINDOW printfs
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
Depends on:
Blocks: fx-noise
  Show dependency treegraph
 
Reported: 2011-07-21 14:48 PDT by Justin Lebar (not reading bugmail)
Modified: 2012-08-10 07:50 PDT (History)
9 users (show)
jruderman: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (4.99 KB, patch)
2011-07-21 14:51 PDT, Justin Lebar (not reading bugmail)
bzbarsky: review-
Details | Diff | Review
Patch v2 (4.78 KB, patch)
2011-08-04 08:27 PDT, Justin Lebar (not reading bugmail)
bzbarsky: review+
Details | Diff | Review

Description Justin Lebar (not reading bugmail) 2011-07-21 14:48:30 PDT
I think it's time for these printfs to go.

Their main utility is to help people find docshell / window leaks.  But I contend that the volume is so high, they're not useful for helping people *notice* leaks.  Once you suspect that there's a docshell / window leak, it's easy to turn on logging.

As it is, it's very difficult to debug sites such as Google+ and Facebook with printf / log messages.  Those sites create and destroy many windows, so your log messages get lost in the noise.

Mounir is working on adding a list of all living windows to about:memory.  As I understand, the plan is to distinguish between windows which are tied to a tab, and orphaned windows, which are likely leaks.  I think this is a great way forward.
Comment 1 Justin Lebar (not reading bugmail) 2011-07-21 14:51:07 PDT
Created attachment 547525 [details] [diff] [review]
Patch v1
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-21 19:00:03 PDT
We've noticed leaks with these recently.  And there are other recent bugs with discussion on just this subject... Please just read it instead of rehashing it in this bug?  :(

Once we have a replacement for these (and I agree that something that only makes noise when you really leak would be better), I'm happy to see them go.

Note that we want something that will just trigger when browsing, without having to actively go open about:memory.  The idea is to notice that there's a memory problem, not to debug it!  We have better tools for the latter.
Comment 3 Justin Lebar (not reading bugmail) 2011-07-22 07:25:57 PDT
> And there are other recent bugs with discussion on just this subject... Please 
> just read it instead of rehashing it in this bug?

Would you mind linking me?  I searched just now and before filing the bug with both bugzilla and google, but didn't find them.  I do recall some discussions on dev.planning, but as I recall, they were just "I want to get rid of them" and "They help us notice leaks" -- I was hoping to take us beyond that.

I'm still not convinced that these notifications are useful to most people, and I don't think it would be so much to ask the few people for whom they are useful to add export NSPR_LOG_MODULES='nsDocShellLeak:5,DOMLeak:5' to their bashrc.

But on the other hand, there's already a lot of debug spew -- what really bothers me isn't that these messages are on by default, but that there's no way to turn them off, and that makes debugging certain sites more difficult.  What if they were on by default but could be turned off by an environment variable?
Comment 4 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-07-22 07:38:04 PDT
I would have no problem with having a way to turn these off.

As far as making people turn them on, unless we do it on tbox things like bug 658738 won't be possible.

I could have sworn we had other recent bugs, but I can't find them either....
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-07-22 07:39:23 PDT
bug 657658?
Comment 6 Justin Lebar (not reading bugmail) 2011-08-04 08:27:38 PDT
Created attachment 550700 [details] [diff] [review]
Patch v2
Comment 7 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-08-04 08:42:17 PDT
Comment on attachment 550700 [details] [diff] [review]
Patch v2

r=me
Comment 8 Justin Lebar (not reading bugmail) 2011-08-04 08:45:53 PDT
Inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/968d17e71c23

Yay.  I'll blog about this and post it to m.d.platform.
Comment 9 Marco Bonardo [::mak] 2011-08-05 08:51:16 PDT
http://hg.mozilla.org/mozilla-central/rev/968d17e71c23
Comment 10 Luke Wagner [:luke] 2012-08-09 18:07:53 PDT
How bad/hard would it be to make "quiet" the default and change the leak tests to set MOZ_QUIET=0?
Comment 11 Justin Lebar (not reading bugmail) 2012-08-10 07:50:58 PDT
(In reply to Luke Wagner [:luke] from comment #10)
> How bad/hard would it be to make "quiet" the default and change the leak
> tests to set MOZ_QUIET=0?

I would be fine with that, but at the time I got a lot of pushback from people who like the spew and didn't want it disabled by default.

FWIW, I always run Firefox from a script which, among other things, sets this env var.

Note You need to log in before you can comment on or make changes to this bug.