Gecko warnings

[Nicholas Nethercote]

Hi,

This is a follow-up to the "Gecko assertions, errors, warnings, et al" thread
(http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/556cb7
debb174cca#).

Output printed to stderr was briefly discussed in that thread, but the
discussion was somewhat lost. So I want to raise the issue again in a more
focused thread.

Warnings are printed to stderr. They don't do anything else. (Unlike
assertions, say, which are caught by fuzzers and some test frameworks.) So
the fact that stderr is spammed with so much output greatly reduces the
value of warnings. For example, when I start-up and shut-down I get 83
lines of stderr output: 33 warnings, 34 lines of DOMWINDOW/DOCSHELL, 14
lines of nsStringStats.

Here are three simple steps that would greatly increase the value of warnings.

- Make the DOMWINDOW/DOCSHELL logging an NSPR logging thing, off by default.

- Make the nsStringStats logging an NSPR logging thing, off by default (but
any leaks could be unconditionally shown, ie. a leak could be a warning).

- Fix the really common warnings -- ones we get on start-up/shut-down, and
on loading really basic sites. (Jesse has done work on this previously,
see https://bugzilla.mozilla.org/show_bug.cgi?id=fx-noise)

Then stderr would by default be very quiet, and warnings would be noticeable
and more useful. Even if the number of lines of output could be reduced
from 83 to a handful would help. Unlike much of what was discussed in the
previous thread, this would only require touching a handful of places in the
code.

Comments welcome.

Nick

[L. David Baron]

On Tuesday 2011-06-14 14:03 +1000, Nicholas Nethercote wrote:
> Here are three simple steps that would greatly increase the value of warnings.
>
> - Make the DOMWINDOW/DOCSHELL logging an NSPR logging thing, off by default.

I'd be more comfortable doing this *after* we get some tests on
tinderbox that we destroy windows and docshells with reasonable
promptness (which also requires getting into a state that such tests
would pass).

> - Make the nsStringStats logging an NSPR logging thing, off by default (but
> any leaks could be unconditionally shown, ie. a leak could be a warning).

Sounds good to me.

>
> - Fix the really common warnings -- ones we get on start-up/shut-down, and
> on loading really basic sites. (Jesse has done work on this previously,
> see https://bugzilla.mozilla.org/show_bug.cgi?id=fx-noise)

Also
https://bugzilla.mozilla.org/buglist.cgi?quicksearch=whiteboard%3A[console-spew]

-David

--
L. David Baron http://dbaron.org/
Mozilla Corporation http://www.mozilla.com/

[Joshua Cranmer]

On 06/14/2011 12:03 AM, Nicholas Nethercote wrote:
> - Make the nsStringStats logging an NSPR logging thing, off by default (but
> any leaks could be unconditionally shown, ie. a leak could be a warning).

I am only going to mention that the stats are useful because they can
help indicate that you may be leaking, since I don't normally run with
leak logs enabled. As long as I can still get notification that "there
was a leak" on shutdown, I'll be happy.

> - Fix the really common warnings -- ones we get on start-up/shut-down, and
> on loading really basic sites. (Jesse has done work on this previously,
> see https://bugzilla.mozilla.org/show_bug.cgi?id=fx-noise)

Just fix the pldhash stuff and I'll be happy. It's been around for
*years* without anyone acting on it, so I question if it's an actually
useful warning at all.

[Dao]

On 14.06.2011 06:18, L. David Baron wrote:
> On Tuesday 2011-06-14 14:03 +1000, Nicholas Nethercote wrote:
>> Here are three simple steps that would greatly increase the value of warnings.
>>
>> - Make the DOMWINDOW/DOCSHELL logging an NSPR logging thing, off by default.
>
> I'd be more comfortable doing this *after* we get some tests on
> tinderbox that we destroy windows and docshells with reasonable
> promptness (which also requires getting into a state that such tests
> would pass).

Yep. I'm very much depending on this data in bug 658738 right now.

[Ehsan Akhgari]

On 11-06-14 12:18 AM, L. David Baron wrote:
> On Tuesday 2011-06-14 14:03 +1000, Nicholas Nethercote wrote:
>> Here are three simple steps that would greatly increase the value of warnings.
>>
>> - Make the DOMWINDOW/DOCSHELL logging an NSPR logging thing, off by default.
>
> I'd be more comfortable doing this *after* we get some tests on
> tinderbox that we destroy windows and docshells with reasonable
> promptness (which also requires getting into a state that such tests
> would pass).

Even then, I think we do want some sort of logs for windows which are
added but not removed on stderr by default. We use this information for
leak detection on Tinderbox, and it's proven to be valuable in the past.

>> - Fix the really common warnings -- ones we get on start-up/shut-down, and
>> on loading really basic sites. (Jesse has done work on this previously,
>> see https://bugzilla.mozilla.org/show_bug.cgi?id=fx-noise)

People have been making these fixes since ever...

Ehsan

[smaug]

On 06/14/2011 07:03 AM, Nicholas Nethercote wrote:
> Hi,
>
> This is a follow-up to the "Gecko assertions, errors, warnings, et al" thread
> (http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/556cb7
> debb174cca#).
>
> Output printed to stderr was briefly discussed in that thread, but the
> discussion was somewhat lost. So I want to raise the issue again in a more
> focused thread.
>
> Warnings are printed to stderr. They don't do anything else. (Unlike
> assertions, say, which are caught by fuzzers and some test frameworks.) So

> the sfact that stderr is spammed with so much output greatly reduces the

> value of warnings. For example, when I start-up and shut-down I get 83
> lines of stderr output: 33 warnings, 34 lines of DOMWINDOW/DOCSHELL, 14
> lines of nsStringStats.
>
> Here are three simple steps that would greatly increase the value of warnings.
>
> - Make the DOMWINDOW/DOCSHELL logging an NSPR logging thing, off by default.
>
> - Make the nsStringStats logging an NSPR logging thing, off by default (but
> any leaks could be unconditionally shown, ie. a leak could be a warning).
>
> - Fix the really common warnings -- ones we get on start-up/shut-down, and
> on loading really basic sites. (Jesse has done work on this previously,
> see https://bugzilla.mozilla.org/show_bug.cgi?id=fx-noise)
>
> Then stderr would by default be very quiet, and warnings would be noticeable
> and more useful. Even if the number of lines of output could be reduced
> from 83 to a handful would help. Unlike much of what was discussed in the
> previous thread, this would only require touching a handful of places in the
> code.

I'm ok with this if there will be clear documentation how to get NSPR
log to stderr, so that I can have the env variable always set to get the
current warnings and DOMWINDOW and DOCSHELL counts. All that
information is really useful, and the little noise there is in the
stderr doesn't disturb me.

-Olli
>
> Comments welcome.
>
> Nick

[Ehsan Akhgari]

We should tread carefully here. There's a large portion of those
warnings which are invaluable in debugging stuff which happens on
Tinderbox for example. I think we should only remove stuff from the
default log if (a) it's something which is being reported by mistake or
(b) if the information is being reported multiple times in different
formats.

Cheers,
Ehsan

[Neil Deakin]

On 11-06-14 2:05 PM, Ehsan Akhgari wrote:

>> I'm ok with this if there will be clear documentation how to get NSPR
>> log to stderr, so that I can have the env variable always set to get the
>> current warnings and DOMWINDOW and DOCSHELL counts. All that
>> information is really useful, and the little noise there is in the
>> stderr doesn't disturb me.
>
> We should tread carefully here. There's a large portion of those
> warnings which are invaluable in debugging stuff which happens on
> Tinderbox for example.

Then that debugging stuff should be enabled on those machines and by the
much smaller number of people that need it.

Note that unlike the other useless debugging info, the
DOMWINDOW/DOCSHELL counting output is printed to stdout not to stderr.
That means I can't redirect it like the rest.

[Boris Zbarsky]

On 6/14/11 2:05 PM, Ehsan Akhgari wrote:
> We should tread carefully here. There's a large portion of those
> warnings which are invaluable in debugging stuff which happens on
> Tinderbox for example.

That specific example should be fixable (e.g. we could always run debug
tbox builds with the relevant env vars said).

> (a) it's something which is being reported by mistake

I think the problem is that different people treat this item
differently: one person's mistake is another's invaluable debugging
tool. :(

-Boris