Comment 1 Simone Gianni 2011-01-10 12:54:43 EST

I'd be happy with a lazy way of creating the thisJoinPoint variable, if it is possible without creating an even bigger performance problem : defining a new class for each join point matched by a global tracing aspect can be far more expensive and slower than passing all arguments every time, since it's a simple Object[] array. Moreover, I don't know if this is true also at bytecode level, but arguments have to be final to be accessible from the LazyJoinPoint implementation.

Since the if() pointcut can access stuff from the context, given a static class (say, Logger) that knows if logging is enabled/disabled and at which level for each class, it's possible to write an if() pointcut that activates the around advice only when the matched join point is in a class that has debug logging activated.

I'll try to write it here, without proper testing, just as an example :

Object around(Object t) : execution(* *.*(...)) && this(t)  && if(Logger.getSettings(t).isDebugEnabled()) {
  // do debug logging, using thisJoinPoint etc...
}

I do understand that this means creating more advice, but consider that it's a good rule to have advice only forward actual "real" work to something else, so it's normal to have many "small" advice that calls more complicated methods somewhere else

Moreover, the proceed can be used inside a closure class (which would be a single class loaded), consider the following (again, "proof of concept", not tested verbatim) :

private interface ProceedClosure() {
  public Object doProceed();
}

private Object performLogging(ProceedClosure pc, JoinPoint jp, LogLevel level) {
  if (level.equals(Debug)) {
    // Do debug logging using jp to extract data
  } else if (........) ....
 
  Object ret = pc.doProceed();

  // do logging on exit, performance logging etc....

  return ret;
}


Object around(final Object t) : execution(* *.*(...)) && this(t)  && if(Logger.getSettings(t).isDebugEnabled()) {
  ProceedClosure pc = new ProceedClosure() {
    public Object doProceed() { 
      return proceed(t);
    }
  }
  return performLogging(pc, thisJoinPoint, Debug);
}

Object around(final Object t) : execution(* *.*(...)) && this(t)  && if(Logger.getSettings(t).isInfoEnabled()) {
  ProceedClosure pc = new ProceedClosure() {
    public Object doProceed() { 
      return proceed(t);
    }
  }
  return performLogging(pc, null, Info);
}


Again, I do understand that having a lazy way of populating the thisJoinPoint variable would simplify things a lot, while still keeping performances down, but I'm afraid that generating an internal class implementing LazyJoinPoint for each matched join point is a pain, while using the above method you can do it "the other way around", creating a limited number of classes.

Comment 2 Marko Umek 2011-02-09 12:43:57 EST

Hello Simone,

> 
> 
> Object around(final Object t) : execution(* *.*(...)) && this(t)  &&
> if(Logger.getSettings(t).isDebugEnabled()) {
>   ProceedClosure pc = new ProceedClosure() {
>     public Object doProceed() { 
>       return proceed(t);
>     }
>   }
>   return performLogging(pc, thisJoinPoint, Debug);
> }
> 

The problem with your solutions, that in the moment the AspectJ-Weaver detects an potential(!) access to thisJoinPoint, the weaver does all the stuff which is necessary to create the thisJoinPoint instance, even if within the advice does not really need this information in some occasions. 

> 
> Again, I do understand that having a lazy way of populating the thisJoinPoint
> variable would simplify things a lot, while still keeping performances down,
> but I'm afraid that generating an internal class implementing LazyJoinPoint for
> each matched join point is a pain, while using the above method you can do it
> "the other way around", creating a limited number of classes.

In Groovy for each Closure literal a class will be created by the compiler. And I've never have had any problem with this or heard about it. And for JAVA 8 the concept of closures (Lambdas) will be part of the language specification, so I expect some clever solutions, to handle the potential problem with the increasing amount of implicit class definitions or the avoidance of them.

Regards Marko