Add MOZ_QUIET env var to suppress ++DOCSHELL, ++DOMWINDOW printfs

RESOLVED FIXED in mozilla8

Status

()

Core
Document Navigation
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
mozilla8
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
Created attachment 547525 [details] [diff] [review]
Patch v1
Attachment #547525 - Flags: review?(bzbarsky)
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.
(Reporter)

Comment 3

6 years ago
> 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?
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....
bug 657658?
Attachment #547525 - Flags: review?(bzbarsky) → review-
(Reporter)

Comment 6

6 years ago
Created attachment 550700 [details] [diff] [review]
Patch v2
Attachment #550700 - Flags: review?(bzbarsky)
(Reporter)

Updated

6 years ago
Attachment #547525 - Attachment is obsolete: true
Comment on attachment 550700 [details] [diff] [review]
Patch v2

r=me
Attachment #550700 - Flags: review?(bzbarsky) → review+
(Reporter)

Comment 8

6 years ago
Inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/968d17e71c23

Yay.  I'll blog about this and post it to m.d.platform.
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/968d17e71c23
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8

Updated

6 years ago
Blocks: 341986

Updated

6 years ago
Summary: Remove ++DOCSHELL, ++DOMWINDOW printfs → Add MOZ_QUIET env var to suppress ++DOCSHELL, ++DOMWINDOW printfs

Updated

6 years ago
Flags: in-testsuite-

Comment 10

5 years ago
How bad/hard would it be to make "quiet" the default and change the leak tests to set MOZ_QUIET=0?
(Reporter)

Comment 11

5 years ago
(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.
You need to log in before you can comment on or make changes to this bug.