[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [aspectj-users] Fixing logger.debug(expensiveObject.toString())

Hi Scott -

Your analysis is correct.  At the time of the logger.debug(..) method-call
join point, the expensive toString() expression has already run.  There is
no good lookahead.

More and more I bite the bullet and use an API that defers rendering
to the log method, e.g.,

  printf("foo %s", expensiveObject)

or more generally

  debug(String format, Object... stuff)

In that case, the expensive operation can be avoided using around advice.  

That refactoring is good where it helps you consolidate rendering operations 
rather that re-engineering toString() for different clients, but it's bad
if you write a god-renderer that knows too much about target classes.

Wes

> ------------Original Message------------
> From: Scott Hayward <shayward@xxxxxxxxxx>
> To: aspectj-users@xxxxxxxxxxx
> Date: Tue, Dec-6-2005 10:12 AM
> Subject: [aspectj-users] Fixing logger.debug(expensiveObject.toString())
>
> Hi,
> 
> I'm looking at a piece of code where there are calls that look like 
> this:
> 
> logger.debug(expensiveObject.toString());
> 
> where the toString() operation is expensive. Unfortunately, the 
> toString()
> call gets evaluated in the production system even though it is never
> written, and the application takes a performance hit. The problem is 
> that
> the code does not check whether debug logging is on. It should really 
> be
> coded like this:
> 
> if logger.isDebug()
> {
>      logger.debug(expensiveObject.toString());
> }
> 
> ----- or -------
> 
> // if the debug method checks the level before doing the 
> object.toString(),
> you could do this
> logger.debug(expensiveObject);
> 
> 
> But, as it happens, that's not what's in the code base.
> 
> My problem is that the toString() call is evaluated before the call to 
> the
> debug method is made. So I can't find a pointcut that will insert 
> advice to
> do a logger.isDebug() test---by the time I know I'm in a debug method 
> (and
> not, say, logger.error), it's too late.
> 
> I could put wrap the toString method of expensive objects with around
> advice and replace the expensive operation if I'm not debugging. But 
> this
> only works if the sole reason to evaluate the expensive toString() is 
> for
> debug logging and *nothing* else. So if the application uses the same
> toString() method elsewhere, the solution cannot be used.
> 
>       public pointcut insideToString() :
>             call(* sandbox.BigUglyLogger.toString());
> 
>       String around() : insideToString()
>       {
>             if (BigUglyLogger.isDebug())
>             {
>                   return proceed();
>             }
>             return 
> thisJoinPointStaticPart.getSignature().toShortString();
>       }
> 
> Have I missed another approach?
> 
> 
> Thanks,
> 
> Scott.
> _______________________________________________________
> Scott Hayward, Senior IT Specialist
> IBM Canada Ltd., Pacific Development Centre
> 4611 Canada Way,  Burnaby, BC, V5G 4X3 CANADA
> Tel (778) 327-7654          Fax (778) 327-3030      4 / PA1 / 4611 / 
> BURN
> email:  shayward@xxxxxxxxxx    Web: http://www.can.ibm.com/pdc
> 
> _______________________________________________
> aspectj-users mailing list
> aspectj-users@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/aspectj-users
>