log4perl-devel

[log4perl-devel] Re: Log::Log4perl

[Mike S. <log...@pe...>]

On Mon, 14 Jul 2003, Kyle R. Burton wrote:

> Many thanks for implementing the Log4j clone.  We're in the process of
> integrating it into our software where I work.  During that process,
> some questions of efficiency came up and we started looking at the
> implementation.  For the actual logging methods, you're generating code
> based on the run-time configuration and leaving out most of the
> conditional checks, which is great.  Looking a bit further, we noticed
> that the is_* methods (is_debug) were not implemented using that
> approach and that raised the question of wether or not they could be.
> Could those methods be created to just return undef or 1?

The Log4perl core uses this snippet to create the is_xxx() methods:

  *{__PACKAGE__ . "::is_$lclevel"} = sub {
    return Log::Log4perl::Level::isGreaterOrEqual($_[0]->level(),
                                                  $$level
                                                  );
  };

which basically ends up in is_xxx() calling two subroutines, and one of
them runs a comparison of two integers. If this is a performance
problem, there's probably some room for us to improve the speed by

* running the comparison that isGreaterOrEqual() is performing right in
  the Logger code (but we'll have to give up this abstraction)

* replacing the level() call to accessing the hash value directly

* maybe providing functions (as you suggested) that return precompiled
  values. The only problem I see right now is that we've got arbitrary
  numbers of user-defined levels and would have to compile an arbitrary
  number of combinations.

Just curious: Did you run benchmarks to get a feel on how much of a
performance penalty is coming out of this?

-- Mike

Mike Schilli
log...@pe...
http://perlmeister.com
http://log4perl.sourceforge.net

[log4perl-devel] Re: Log::Log4perl (2)

[Mike S. <log...@pe...>]

On Mon, 14 Jul 2003, Kyle R. Burton wrote:

> Looking a bit further, we noticed
> that the is_* methods (is_debug) were not implemented using that
> approach and that raised the question of wether or not they could be.
> Could those methods be created to just return undef or 1?

Actually, please ignore my previous email, I looked at the code again
found an easy patch that'll do exactly what you suggested. I've run a
couple of benchmarks -- it improves performance of all is_xxx() methods
by a whopping 300%. Here's the fix -- Kevin and Erik, can you guys please
double-check if I've overlooked anything?

Index: Logger.pm
===================================================================
RCS file: /cvsroot/log4perl/Log-Log4perl/lib/Log/Log4perl/Logger.pm,v
retrieving revision 1.51
diff -a -u -r1.51 Logger.pm
--- Logger.pm   6 Jul 2003 21:39:29 -0000       1.51
+++ Logger.pm   15 Jul 2003 07:32:25 -0000
@@ -157,10 +157,12 @@
                                                   )) {
             print "  ($priority{$levelname} <= $level)\n"
                   if DEBUG;
-            $self->{$levelname} = $coderef;
+            $self->{$levelname}      = $coderef;
+            $self->{"is_$levelname"} = sub { 1 };
         }else{
             print "  ($priority{$levelname} > $level)\n" if DEBUG;
-            $self->{$levelname} = $noop;
+            $self->{$levelname}      = $noop;
+            $self->{"is_$levelname"} = sub { 0 };
         }

         print("  Setting [$self] $self->{category}.$levelname to ",
@@ -574,10 +576,11 @@
         $_[0]->{$level}->(@_, $level);
      };

-  *{__PACKAGE__ . "::is_$lclevel"} = sub {
-    return Log::Log4perl::Level::isGreaterOrEqual($_[0]->level(),
-                                                 $$level
-                                                 );
+  *{__PACKAGE__ . "::is_$lclevel"} = sub {
+      $_[0]->{"is_" . $level}->();
+#    return Log::Log4perl::Level::isGreaterOrEqual($_[0]->level(),
+#                                                $$level
+#                                                );
   };

   use strict qw(refs);


-- Mike

Mike Schilli
log...@pe...
http://perlmeister.com
http://log4perl.sourceforge.net

Re: [log4perl-devel] Re: Log::Log4perl (2)

[Erik W. S. <er...@se...>]

Sweet... nice bit o' work there. And yeah, much speedier. :)

Truth be told, never worried much about performance for most things 
Perl, as the interpreter and RE isn't the speediest thing in the planet.

-e


Mike Schilli wrote:

>On Mon, 14 Jul 2003, Kyle R. Burton wrote:
>
>  
>
>>Looking a bit further, we noticed
>>that the is_* methods (is_debug) were not implemented using that
>>approach and that raised the question of wether or not they could be.
>>Could those methods be created to just return undef or 1?
>>    
>>
>
>Actually, please ignore my previous email, I looked at the code again
>found an easy patch that'll do exactly what you suggested. I've run a
>couple of benchmarks -- it improves performance of all is_xxx() methods
>by a whopping 300%. Here's the fix -- Kevin and Erik, can you guys please
>double-check if I've overlooked anything?
>
>Index: Logger.pm
>===================================================================
>RCS file: /cvsroot/log4perl/Log-Log4perl/lib/Log/Log4perl/Logger.pm,v
>retrieving revision 1.51
>diff -a -u -r1.51 Logger.pm
>--- Logger.pm   6 Jul 2003 21:39:29 -0000       1.51
>+++ Logger.pm   15 Jul 2003 07:32:25 -0000
>@@ -157,10 +157,12 @@
>                                                   )) {
>             print "  ($priority{$levelname} <= $level)\n"
>                   if DEBUG;
>-            $self->{$levelname} = $coderef;
>+            $self->{$levelname}      = $coderef;
>+            $self->{"is_$levelname"} = sub { 1 };
>         }else{
>             print "  ($priority{$levelname} > $level)\n" if DEBUG;
>-            $self->{$levelname} = $noop;
>+            $self->{$levelname}      = $noop;
>+            $self->{"is_$levelname"} = sub { 0 };
>         }
>
>         print("  Setting [$self] $self->{category}.$levelname to ",
>@@ -574,10 +576,11 @@
>         $_[0]->{$level}->(@_, $level);
>      };
>
>-  *{__PACKAGE__ . "::is_$lclevel"} = sub {
>-    return Log::Log4perl::Level::isGreaterOrEqual($_[0]->level(),
>-                                                 $$level
>-                                                 );
>+  *{__PACKAGE__ . "::is_$lclevel"} = sub {
>+      $_[0]->{"is_" . $level}->();
>+#    return Log::Log4perl::Level::isGreaterOrEqual($_[0]->level(),
>+#                                                $$level
>+#                                                );
>   };
>
>   use strict qw(refs);
>
>
>-- Mike
>
>Mike Schilli
>log...@pe...
>http://perlmeister.com
>http://log4perl.sourceforge.net
>
>
>-------------------------------------------------------
>This SF.Net email sponsored by: Parasoft
>Error proof Web apps, automate testing & more.
>Download & eval WebKing and get a free book.
>www.parasoft.com/bulletproofapps1
>_______________________________________________
>log4perl-devel mailing list
>log...@li...
>https://lists.sourceforge.net/lists/listinfo/log4perl-devel
>
>  
>

[log4perl-devel] Re: Log::Log4perl (2)

[Kyle R. B. <mo...@vo...>]

> > Looking a bit further, we noticed
> > that the is_* methods (is_debug) were not implemented using that
> > approach and that raised the question of wether or not they could be.
> > Could those methods be created to just return undef or 1?
> 
> Actually, please ignore my previous email, I looked at the code again
> found an easy patch that'll do exactly what you suggested. I've run a
> couple of benchmarks -- it improves performance of all is_xxx() methods
> by a whopping 300%. Here's the fix -- Kevin and Erik, can you guys please
> double-check if I've overlooked anything?

That's great!  Thanks for the quick response and action.  To answer your
earlier question, no we haven't yet run benchmarks, we were only considering
the effect of having hundreds of if(is_debug) calls in our codebase.  When
we started worrying about the effect of log4perl integration, we wonderd
about the implementation - and when we looked, we were glad to see that
most of the expensive run-time operations were factored out to configuration
and/or construction time.  The is_* tests just seemed to not follow the
design of the rest of the code.

Any idea when a new release might happen?  


Thanks again,

Kyle

> Index: Logger.pm
> ===================================================================
> RCS file: /cvsroot/log4perl/Log-Log4perl/lib/Log/Log4perl/Logger.pm,v
> retrieving revision 1.51
> diff -a -u -r1.51 Logger.pm
> --- Logger.pm   6 Jul 2003 21:39:29 -0000       1.51
> +++ Logger.pm   15 Jul 2003 07:32:25 -0000
> @@ -157,10 +157,12 @@
>                                                    )) {
>              print "  ($priority{$levelname} <= $level)\n"
>                    if DEBUG;
> -            $self->{$levelname} = $coderef;
> +            $self->{$levelname}      = $coderef;
> +            $self->{"is_$levelname"} = sub { 1 };
>          }else{
>              print "  ($priority{$levelname} > $level)\n" if DEBUG;
> -            $self->{$levelname} = $noop;
> +            $self->{$levelname}      = $noop;
> +            $self->{"is_$levelname"} = sub { 0 };
>          }
> 
>          print("  Setting [$self] $self->{category}.$levelname to ",
> @@ -574,10 +576,11 @@
>          $_[0]->{$level}->(@_, $level);
>       };
> 
> -  *{__PACKAGE__ . "::is_$lclevel"} = sub {
> -    return Log::Log4perl::Level::isGreaterOrEqual($_[0]->level(),
> -                                                 $$level
> -                                                 );
> +  *{__PACKAGE__ . "::is_$lclevel"} = sub {
> +      $_[0]->{"is_" . $level}->();
> +#    return Log::Log4perl::Level::isGreaterOrEqual($_[0]->level(),
> +#                                                $$level
> +#                                                );
>    };
> 
>    use strict qw(refs);
> 
> 
> -- Mike
> 
> Mike Schilli
> log...@pe...
> http://perlmeister.com
> http://log4perl.sourceforge.net

-- 

------------------------------------------------------------------------------
Wisdom and Compassion are inseparable.
        -- Christmas Humphreys
mo...@vo...                            http://www.voicenet.com/~mortis
------------------------------------------------------------------------------

[log4perl-devel] Re: Log::Log4perl (2)

[Mike S. <msc...@ao...>]

On Tue, 15 Jul 2003, Kyle R. Burton wrote:

> Any idea when a new release might happen?

Later this week, we should have the 0.36 release ready on
log4perl.sourceforge.net, which will hit CPAN next week.

-- Mike

Mike Schilli
msc...@ao...
Unified Preferences Engineering