Because

1. The if condition has a cost (3 times more than nothing according to 
Mathieu's reference[1])
2. and is not consistently used.
3. Using "Beautiful Logger" could be a better solution: 
https://github.com/forax/beautiful_logger
   drawbacks:
     *   needs Java 11 to build. But will we use Java 8 forever? Can run with 
Java 8.
     * "put more pressure to the JITs so it may lengthen the time to steady state of 
an application."

[1] https://www.youtube.com/watch?v=z5UkoLaW6ME (French)

Jacques

Le 11/12/2018 à 12:15, Taher Alkhateeb a écrit :
We already agreed and decided to keep the if condition. I'm not sure
why this subject is being reopened.
On Mon, Dec 10, 2018 at 9:49 PM Jacques Le Roux
<jacques.le.r...@les7arts.com> wrote:
The question is quite simple. It's about always using or not the /if 
(Debug.levelOn()) {/ expression for the info and debug level as suggests 
Michael.

Or only using /if/ /(Debug.verboseOn()) { /expression as I recommend for the 
reasons explained.

I think we all agree the /if/ /(Debug.verboseOn()) { /should always be used and 
enforced when missing.

I put all the details/arguments to easily refer to them if needed.

Also Mathieu suggested to Michael to have a look at "Beautiful Logger" because 
he is concerned by the logging performance. We could also have a look
at it, to see if it could be integrated OOTB.

HTH

Jacques

Le 10/12/2018 à 18:02, Taher Alkhateeb a écrit :
I didn't understand the exact decision required? To do what vs what?
On Mon, Dec 10, 2018 at 7:43 PM Jacques Le Roux
<jacques.le.r...@les7arts.com> wrote:
Hi,

After OFBIZ-10052, OFBIZ-10448, and OFBIZ-10697 I think it's time to have 
another discussion following https://markmail.org/message/mplvusuqn7oshl4v
which was one year ago.

In OFBIZ-10697 I wrote:

      <<OK let me explain my point.

      By default OOTB debug.properties sets all debug levels to be used but 
verbose. So there is no point checking other levels than verbose to see if
      they are used, they are anyway. So OOTB only checking verbose is needed. 
That's what developers need.

      Now you invoked production where a different debug.properties setting 
would be used. Sincerely I'd never, never, remove the other levels than
      verbose from a production setting (as it's OOTB). I'd even be quite 
reluctant to remove any of those levels from a production site running for
      years! *You never know what can happen*, and those debug levels are your 
only lifebelt in case of issues, small and big ones.

      That's my point, and that's why I see checking if a level is used as 
premature optimisation but for verbose. We need them anyway and IMO setting
      false for any but verbose in debug.properties in production would be conceited if 
not "suicidal" . But maybe you have use cases I ignore?

      Anway, I'll start a discussion in dev ML about this point. Like I said above 
and in OFBIZ-10448 <https://issues.apache.org/jira/browse/OFBIZ-10448>:

          I'd be all for removing the 312 useless cases but not the "if 
(Debug.verboseOn())">>

To what Michael answered

      <<We have several projects with huge traffic and you certainly won't run 
them on debug level info, verbose or debug.
      In my view, at least these 3 debug levels should be checked. At least, existing 
checks should not be removed.>>

I think Michael's point is perfectly valid. So I answered:

      <<I had not the same experience, you are lucky to not cross any issue in 
production needing an info or debug level.
      OK then we need to see things otherwise and rather enforce the checks to 
these 2 levels at least in all the source (Java and Groovy). That's what
      I'll ask in the convo to come in dev ML.>>

So what are your opinion about that? Should we enforce as suggest Michael or 
should we remove for the reasons I wrote.

I'd prefer to be consistent. So either we enforce the checks to the info, 
verbose and debug levels. Either we only keep the verbose checks.

Finally Mathieu has added a grain of salt:

      <<Hello Michael Brohl 
<https://issues.apache.org/jira/secure/ViewProfile.jspa?name=mbrohl>,

      If you care about the impact of loggers on performance you should take a look 
at the Beautiful Logger <https://github.com/forax/beautiful_logger>
      library by Remi Forax which implements a truly transparent logger. In a recent 
Devoxx talk <https://www.youtube.com/watch?v=z5UkoLaW6ME> (in
      french) he showed that even doing if (Debug.logXXX()) has a 3x slowdown 
compared to a no-op.>>

So maybe we should have a closer look at  Beautiful Logger before taking any decision? 
Note that it uses JitPack, hence seems to need "Java 11 to build".

Thanks

Jacques

Reply via email to