Enable DVLOG on non official release build based on the parameter passed for InitLogging.

Enable DVLOG on non official release build based on the parameter passed for InitLogging.

For chrome this would be enabled/disabled based on command line flag.


BUG=101424
TEST=base_unittests.exe

[lipalani1, 2011-11-08 00:33:41]

Will - Please review.

I was talking to Fred about this and we decided it is better to get it in sooner
rather than waiting for him to come back.

This came out of the discussion we had with Brett about cutting down the cost of
logging on official builds.

[rvargas (doing something else), 2011-11-08 02:21:50]

Isn't this counter-intuitive?

I believe there is a strong reason for having DCHECK enabled for a release, but
non-official build (double check all assertions on a fast build, so we can use
it on the bots).

However, if we end up enabling DLOG for release builds, the whole D part becomes
meaningless. I'm aware of the chromium-dev thread about removing logs, but this
still feels wrong.

I don't see that strong reason to enable this behavior.

To put it another way: current DLOG statements were added under the assumption
that they would not be present on release builds.

[lipalani, 2011-11-08 02:44:39]

Hi,
Just to be clear this does NOT enable dlog for release builds. It only
enables them if chrome is run with a command line switch on release builds.
So the assumption of current DLOG statements are not way off the mark.
However if there are cases where we run explicitly with the command line
flag and there are instances where executing those DLOG statements are
harmful, can you give an example of that.

Thanks!


On Mon, Nov 7, 2011 at 6:21 PM, <rvargas@chromium.org> wrote:

> Isn't this counter-intuitive?
>
> I believe there is a strong reason for having DCHECK enabled for a
> release, but
> non-official build (double check all assertions on a fast build, so we can
> use
> it on the bots).
>
> However, if we end up enabling DLOG for release builds, the whole D part
> becomes
> meaningless. I'm aware of the chromium-dev thread about removing logs, but
> this
> still feels wrong.
>
> I don't see that strong reason to enable this behavior.
>
> To put it another way: current DLOG statements were added under the
> assumption
> that they would not be present on release builds.
>
>
http://codereview.chromium.**org/8467028/<http://codereview.chromium.org/8467...
>

[rvargas (doing something else), 2011-11-08 03:32:59]

On 2011/11/08 02:44:39, lipalani wrote:
> Hi,
> Just to be clear this does NOT enable dlog for release builds. It only
> enables them if chrome is run with a command line switch on release builds.

Yes, I understand that... that's exactly how DCHECK works.

> So the assumption of current DLOG statements are not way off the mark.
> However if there are cases where we run explicitly with the command line
> flag and there are instances where executing those DLOG statements are
> harmful, can you give an example of that.

harmful as in something will explode? never... after all they are there in debug
builds today. Are they out of my way? hey, even if there was no command line
they will be out of my way because I usually don't enable logging, nor do I go
looking for an output file somewhere. That doesn't mean I'm not bothered by all
the log lines that we spew to stdout just by running unit tests.

> 
> Thanks!
> 
> 
> On Mon, Nov 7, 2011 at 6:21 PM, <mailto:rvargas@chromium.org> wrote:
> 
> > Isn't this counter-intuitive?
> >
> > I believe there is a strong reason for having DCHECK enabled for a
> > release, but
> > non-official build (double check all assertions on a fast build, so we can
> > use
> > it on the bots).
> >
> > However, if we end up enabling DLOG for release builds, the whole D part
> > becomes
> > meaningless. I'm aware of the chromium-dev thread about removing logs, but
> > this
> > still feels wrong.
> >
> > I don't see that strong reason to enable this behavior.
> >
> > To put it another way: current DLOG statements were added under the
> > assumption
> > that they would not be present on release builds.
> >
> >
>
http://codereview.chromium.**org/8467028/%3Chttp://codereview.chromium.org/84...>
> >

[lipalani, 2011-11-08 03:42:48]

Not sure I follow your second comment. But to reiterate the reason:
There are bugs that occur only in release builds. In those cases this will
allow us to run with vmodule command line and enable verbose logging on
certain files and get DVLOG statements out on only those files. This will
not be a uncontrolled spew.

Hope that clarifies things.

Thanks!

On Mon, Nov 7, 2011 at 7:32 PM, <rvargas@chromium.org> wrote:

> On 2011/11/08 02:44:39, lipalani wrote:
>
>> Hi,
>> Just to be clear this does NOT enable dlog for release builds. It only
>> enables them if chrome is run with a command line switch on release
>> builds.
>>
>
> Yes, I understand that... that's exactly how DCHECK works.
>
>
>  So the assumption of current DLOG statements are not way off the mark.
>> However if there are cases where we run explicitly with the command line
>> flag and there are instances where executing those DLOG statements are
>> harmful, can you give an example of that.
>>
>
> harmful as in something will explode? never... after all they are there in
> debug
> builds today. Are they out of my way? hey, even if there was no command
> line
> they will be out of my way because I usually don't enable logging, nor do
> I go
> looking for an output file somewhere. That doesn't mean I'm not bothered
> by all
> the log lines that we spew to stdout just by running unit tests.
>
>
>  Thanks!
>>
>
>
>  On Mon, Nov 7, 2011 at 6:21 PM, <mailto:rvargas@chromium.org> wrote:
>>
>
>  > Isn't this counter-intuitive?
>> >
>> > I believe there is a strong reason for having DCHECK enabled for a
>> > release, but
>> > non-official build (double check all assertions on a fast build, so we
>> can
>> > use
>> > it on the bots).
>> >
>> > However, if we end up enabling DLOG for release builds, the whole D part
>> > becomes
>> > meaningless. I'm aware of the chromium-dev thread about removing logs,
>> but
>> > this
>> > still feels wrong.
>> >
>> > I don't see that strong reason to enable this behavior.
>> >
>> > To put it another way: current DLOG statements were added under the
>> > assumption
>> > that they would not be present on release builds.
>> >
>> >
>>
>
> http://codereview.chromium.****org/8467028/%3Chttp://coderevi**
> ew.chromium.org/8467028/ <http://codereview.chromium.org/8467028/>>
>
>> >
>>
>
>
>
>
http://codereview.chromium.**org/8467028/<http://codereview.chromium.org/8467...
>

[lipalani1, 2011-11-08 22:39:10]

Friendly ping for the review!!
Thanks!

[rvargas (doing something else), 2011-11-10 19:43:21]

I still don't believe that this is something that we should do.

If we can provide a release but not official build, we can also add very
specific code to debug whatever is happening (being that regular LOGs, CHECKs,
dialogs or whatever), and remove that debugging crust after the bug is fixed.

I don't believe the long term value of debugging log statements(*) to be high
enough to justify their existence, much less the complexity and
counter-intuitiveness of a general mechanism to enable them to live on release
builds.

However, I'm not a base owner, and that means that I could not approve this CL
even if I wanted it to land.

(*) I don't mean we should get rid of DLOGS, I mean the type of messages that
someone has to add to debug something on the bots (for instance)... something so
uncommon that it's priority goes to DVLOG with some number. If it has that
priority for real (even DLOG(INFO)), it should go away on release.

[lipalani, 2011-11-10 20:02:14]

I had a chat with Ricardo as well. It looks like his main point of concern
is that our logging system is heavy weight and slow.

However both of us do agree that it is important to see what is happening
inside a release build if a need arises.

Even if we build a light and fast logging system we would still need a
mechanism for enabling/disabling logging in release builds. I don't think
there is disagreement there. And this CL provides the way for that.
Although when we re-design the logging system the guts of it might change
the consumers of it can still rely on the fact that DVLOG is on by default
in debug, off by default in non release and can be enabled as needed, and
stripped out in official builds.(So consumers dont have to change much).

Also in the past VLOGs have proved really effective to debug some bot only
failures(at least I can attest to one instance in which it took us a week
to track down a bot only failure and we tracked it down primarily by
enabling vlog on certain files and letting it run a couple of days on the
bots). Until we redesign the logging system it does not make sense to fly
blind with no logs for non official release builds.

Thanks!
regards,
Lingesh

On Thu, Nov 10, 2011 at 11:43 AM, <rvargas@chromium.org> wrote:

> I still don't believe that this is something that we should do.
>
> If we can provide a release but not official build, we can also add very
> specific code to debug whatever is happening (being that regular LOGs,
> CHECKs,
> dialogs or whatever), and remove that debugging crust after the bug is
> fixed.
>
> I don't believe the long term value of debugging log statements(*) to be
> high
> enough to justify their existence, much less the complexity and
> counter-intuitiveness of a general mechanism to enable them to live on
> release
> builds.
>
> However, I'm not a base owner, and that means that I could not approve
> this CL
> even if I wanted it to land.
>
> (*) I don't mean we should get rid of DLOGS, I mean the type of messages
> that
> someone has to add to debug something on the bots (for instance)...
> something so
> uncommon that it's priority goes to DVLOG with some number. If it has that
> priority for real (even DLOG(INFO)), it should go away on release.
>
>
http://codereview.chromium.**org/8467028/<http://codereview.chromium.org/8467...
>

[rvargas (doing something else), 2011-11-10 20:35:10]

On 2011/11/10 20:02:14, lipalani wrote:
> I had a chat with Ricardo as well. It looks like his main point of concern
> is that our logging system is heavy weight and slow.
> 
> However both of us do agree that it is important to see what is happening
> inside a release build if a need arises.
> 
> Even if we build a light and fast logging system we would still need a
> mechanism for enabling/disabling logging in release builds. I don't think
> there is disagreement there. And this CL provides the way for that.

Nope. We also disagree there. Having macros that are meant to be debug-only be
now compiled in release builds but somewhat disabled through a command line
option (or lack of) is counter intuitive, and adds another layer of complexity
to logging.h (even if at this point the change looks quite trivial there).

I don't think the crux of the issue is some future redesign of the logging
system. It is what I tried to express before: debugging specific issues on the
bots should be a one time add / remove thing. If someone finds him(her)self
doing that all the time for the same logs, then that points to either the lack
of a better troubleshooting method in that part of the code, or the need for a
LOG (or VLOG) enabled all the time (not Dxx).

> Although when we re-design the logging system the guts of it might change
> the consumers of it can still rely on the fact that DVLOG is on by default
> in debug, off by default in non release and can be enabled as needed, and
> stripped out in official builds.(So consumers dont have to change much).
> 
> Also in the past VLOGs have proved really effective to debug some bot only
> failures(at least I can attest to one instance in which it took us a week
> to track down a bot only failure and we tracked it down primarily by
> enabling vlog on certain files and letting it run a couple of days on the
> bots). Until we redesign the logging system it does not make sense to fly
> blind with no logs for non official release builds.
> 
> Thanks!
> regards,
> Lingesh
> 
> On Thu, Nov 10, 2011 at 11:43 AM, <mailto:rvargas@chromium.org> wrote:
> 
> > I still don't believe that this is something that we should do.
> >
> > If we can provide a release but not official build, we can also add very
> > specific code to debug whatever is happening (being that regular LOGs,
> > CHECKs,
> > dialogs or whatever), and remove that debugging crust after the bug is
> > fixed.
> >
> > I don't believe the long term value of debugging log statements(*) to be
> > high
> > enough to justify their existence, much less the complexity and
> > counter-intuitiveness of a general mechanism to enable them to live on
> > release
> > builds.
> >
> > However, I'm not a base owner, and that means that I could not approve
> > this CL
> > even if I wanted it to land.
> >
> > (*) I don't mean we should get rid of DLOGS, I mean the type of messages
> > that
> > someone has to add to debug something on the bots (for instance)...
> > something so
> > uncommon that it's priority goes to DVLOG with some number. If it has that
> > priority for real (even DLOG(INFO)), it should go away on release.
> >
> >
>
http://codereview.chromium.**org/8467028/%3Chttp://codereview.chromium.org/84...>
> >

[willchan no longer on Chromium, 2011-11-10 20:56:26]

Thanks for chiming in guys. I haven't even read this, but it sounds like
this change is still contentious. I'd like a bit more consensus or at least
rvargas's opinion being reduced to "I disagree but don't care enough to
fight".

I would also like this change's discussion to move back to chromium-dev.
I'm going to copy/paste discussion back there.

On Thu, Nov 10, 2011 at 12:35 PM, <rvargas@chromium.org> wrote:

> On 2011/11/10 20:02:14, lipalani wrote:
>
>> I had a chat with Ricardo as well. It looks like his main point of concern
>> is that our logging system is heavy weight and slow.
>>
>
>  However both of us do agree that it is important to see what is happening
>> inside a release build if a need arises.
>>
>
>  Even if we build a light and fast logging system we would still need a
>> mechanism for enabling/disabling logging in release builds. I don't think
>> there is disagreement there. And this CL provides the way for that.
>>
>
> Nope. We also disagree there. Having macros that are meant to be
> debug-only be
> now compiled in release builds but somewhat disabled through a command line
> option (or lack of) is counter intuitive, and adds another layer of
> complexity
> to logging.h (even if at this point the change looks quite trivial there).
>
> I don't think the crux of the issue is some future redesign of the logging
> system. It is what I tried to express before: debugging specific issues on
> the
> bots should be a one time add / remove thing. If someone finds him(her)self
> doing that all the time for the same logs, then that points to either the
> lack
> of a better troubleshooting method in that part of the code, or the need
> for a
> LOG (or VLOG) enabled all the time (not Dxx).
>
>
>  Although when we re-design the logging system the guts of it might change
>> the consumers of it can still rely on the fact that DVLOG is on by default
>> in debug, off by default in non release and can be enabled as needed, and
>> stripped out in official builds.(So consumers dont have to change much).
>>
>
>  Also in the past VLOGs have proved really effective to debug some bot only
>> failures(at least I can attest to one instance in which it took us a week
>> to track down a bot only failure and we tracked it down primarily by
>> enabling vlog on certain files and letting it run a couple of days on the
>> bots). Until we redesign the logging system it does not make sense to fly
>> blind with no logs for non official release builds.
>>
>
>  Thanks!
>> regards,
>> Lingesh
>>
>
>  On Thu, Nov 10, 2011 at 11:43 AM, <mailto:rvargas@chromium.org> wrote:
>>
>
>  > I still don't believe that this is something that we should do.
>> >
>> > If we can provide a release but not official build, we can also add very
>> > specific code to debug whatever is happening (being that regular LOGs,
>> > CHECKs,
>> > dialogs or whatever), and remove that debugging crust after the bug is
>> > fixed.
>> >
>> > I don't believe the long term value of debugging log statements(*) to be
>> > high
>> > enough to justify their existence, much less the complexity and
>> > counter-intuitiveness of a general mechanism to enable them to live on
>> > release
>> > builds.
>> >
>> > However, I'm not a base owner, and that means that I could not approve
>> > this CL
>> > even if I wanted it to land.
>> >
>> > (*) I don't mean we should get rid of DLOGS, I mean the type of messages
>> > that
>> > someone has to add to debug something on the bots (for instance)...
>> > something so
>> > uncommon that it's priority goes to DVLOG with some number. If it has
>> that
>> > priority for real (even DLOG(INFO)), it should go away on release.
>> >
>> >
>>
>
> http://codereview.chromium.****org/8467028/%3Chttp://coderevi**
> ew.chromium.org/8467028/ <http://codereview.chromium.org/8467028/>>
>
>> >
>>
>
>
>
>
http://codereview.chromium.**org/8467028/<http://codereview.chromium.org/8467...
>