309845 - Suppress WEBSHELL debug spew by default

Status: RESOLVED WONTFIX

(Core :: DOM: Navigation, defect)

Description

[Darin Fisher]

In an effort to make the console output of a debug build less noisy, I'd like
to make the "WEBSHELL = N" spew use NSPR logging.

Comment 1

[Darin Fisher]

Attachment: v1 patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

David, are you still interested in these numbers?  That is, should we have a
DEBUG_something ifdef that logs them unconditionally?

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

IMO, these should still be on for everybody.  Leaks for anything outside of the
bloaturls pageloader don't get much attention.

Comment 4

[Darin Fisher]

> IMO, these should still be on for everybody.  Leaks for anything outside of 
> the bloaturls pageloader don't get much attention.

I can appreciate that awareness is important, but not everyone who uses a debug
build will understand what these mean or care.  I'd really like to reduce our
debug spew to only include warnings and errors (assertions).  The rest of the
output is just noise to the majority of developers.  I think this will help make
our codebase friendlier to the newcomer.  But, maybe I'm wrong about that :-/

Can we devise a test that would determine if a WEBSHELL has leaked when it
should not?  Then, NS_ERROR when it is?  That's probably hard to do. 
Personally, I don't know when WEBSHELL's count should go to zero, so it doesn't
do me much good to see all the WEBSHELL spew.  If I did know what it means, then
perhaps I'd choose to see it, which is why conditional logging is nice.

One more point:  printf logging doesn't mix with NSPR logging unless NSPR
logging is dumped to stdout.  It'd be nice if all of our logging optionally went
to the NSPR log file.  NS_WARNING, NS_ERROR, and NS_ASSERTION all do, so it
seems that using printf in the raw for important stuff is just not all that great.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

There are a bunch of people who test debug builds but don't do much development
who do know what this means, and start raising alarms when it goes bad.  I don't
have the time or energy to be the only person who cares about leaks, and unless
that substantially changes (i.e., I see other people fixing architectural leak
bugs and doing regular leak testing), I'm going to insist that this stays.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 197240 [details] [diff] [review]
v1 patch

per the comments from dbaron, minussing patch

Comment 7

[Darin Fisher]

dbaron:

Wait a minute.  If there are a bunch of people who regularly track this debug
output, then why does it not follow that the same bunch of people could just as
easily define NSPR_LOG_MODULES in their environment to restore the debug output?
 Moreover, I could imagine that we might want to compile this log module into
release builds (FORCE_PR_LOG) to make it even easier for people to see if these
leaks are happening.  Extension developers might appreciate the data as well,
and they might not want to use a debug Firefox build in order to test their
extensions.  I know a number of extension authors who are in that boat.  I also
know some "AJAX" developers who would like to see this kind of data, and that
audience most certainly is not going to run a debug build if they can help it.

Basically, I don't understand how the printf output is useful to anyone who
doesn't already know 1) to look for that output and 2) to make sense of it.  I
suspect this output is only useful to a very select bunch of people.  If the
output were a message like "warning: memory leak detected," then the story would
be different.

Comment 8

[Darin Fisher]

WONTFIX since this is apparently not a very popular idea.