[aspectj-users] Language: support for pertype aspects

["Jim Hugunin" <lists@xxxxxxxxxxx>]

https://bugs.eclipse.org/bugs/show_bug.cgi?id=48082

This is a feature request for pertype aspect instantiation.  The initial
motivating example for this feature is to support system-wide logging
policies.  Usually these policies are implemented as follows:

class Foo {
    static final Log log = new Log(Foo.class);
    
    public void someMethod() {
        log.enterMethod("someMethod");
        ....
    }
    ...
}

If this policy is captured by an aspect in AspectJ-1.1, a HashMap must be
used to hold the logs for each class.  The code looks something like this:

aspect Logger {
    private final IdentityHashMap logs = new IdentityHashMap();

    before(): staticinitialization(com.boxes..*) {
        Class thisClass = 
            thisJoinPointStaticPart.getSignature().getDeclaringType();
        logs.put(thisClass, new Log(thisClass));
    }

    before(): execution(public * *(..)) && within(com.boxes..*) {
        Class thisClass = 
            thisJoinPointStaticPart.getSignature().getDeclaringType();
        Log log = (Log)logs.get(thisClass);
        log.enterMethod(thisJoinPoint.getSignature().getName());
    }
}

While this code is a little bit ugly, that's not the primary concern here.
The issue is that this use of a HashMap at every method execution can add a
very large overhead to the application.  For most applications this is not
acceptable and the hand-coded version of logging shown previously must be
used.

Adding a pertype aspect would let us implement this concern both more
clearly and more efficiently, i.e.    

aspect Logger pertype(com.boxes..*) {
    public Log log;
        
    before(): staticinitialization(com.boxes..*) {
        log = new Log(thisJoinPoint.getSignature().getDeclaringType());
    }

    before(): execution(public * *(..)) {
        log.enterMethod(thisJoinPoint.getSignature().getName());
    }
}

There are some design difficulties involved in getting this design just
right.  However, before we can work out the detailed design of this feature,
we need to have several additional use cases for it.  Here's one more
example that starts from a very generic caching aspect:

public abstract aspect Caching {
    private Object cachedValue = null;
    
    protected abstract pointcut cachePoints();

    Object around(): cachePoints() {
        if (cachedValue == null) cachedValue = proceed();
        return cachedValue;
    }
}

/* Add caching to a static method of no args whose result never changes. 
 */
aspect MyCache pertype(MyClass) {
    protected pointcut cachePoints():
        execution(static * MyClass.cacheThisMethod());
}


Do you as AspectJ users have other use cases for a pertype aspect?  If so,
please send them to this list in as concrete form as possible so we can
discuss them.  If we don't get enough more compelling examples for this
feature it is unlikely to be included in AspectJ-1.2.

-Jim



-------------------------------------------------
The rest of this message goes into more details of the design of pertype
aspects.  We should only discuss these details if we can find a few more
compelling use cases.

There are three main issues to resolve in the design of this feature:

1. Should we use pertype(TypePattern) or pertype(PointcutDesignator)
2. Should the aspect instance be active for inner types of the type it is
bound to (like within works)?
3. What should the signature be of the aspect's constructor?


Let's see how the two options in issue #1 look when we consider the three
things that must be specified for any per clause in AspectJ.
i. When is an instance of the aspect created?
ii. What is the instance bound to?
iii. What is the implicit pointcut for advice in the aspect?

Option A: pertype(TypePattern)
i. an aspect instance is created just before the staticinitialization of any
type T matching the TypePattern
ii. the instance is bound to the type T
iii. the advice will run for all code that matches within(T)
Note: With inner classes it is possible to have multiple instances of a
given aspect whose advice could run at any particular join point.  In this
case we will choose the aspect instance bound to the inner-most type.

Option B: pertype(PointcutDesignator)
i. an aspect instance is created for each join point that matches
PointcutDesignator whenever the statically enclosing type (T) at that join
point doesn't already have an instance of this aspect bound
ii. the instance is bound to the type T
iii. as above, the advice will run for all code that matches within(T)

Note: Option B is consistent with all of the existing per clauses in AspectJ
that take a PointcutDesignator to specify their binding.  All of these
clauses are named to indicate their implicit pointcut, i.e. perthis(pcd) has
an implicit pointcut of this(T).  To be consistent with this naming
convention, under option B we should use the name 'perwithin(...)'.


-----------------------------------------------------------
Inner and anonymous types

aspect Logging pertype(com.boxes..*) { . }

This must be created for all types in com.boxes package, all sub packages
and all inner types.  This will create a new logger for all anonymous and
inner classes.  This behavior doesn't match any logging policy document that
I've ever read.

aspect Logging pertype(!@InnerType com.boxes..*) { . }

See the previous message about modifiers on TypePatterns for an explanation
of @InnerType.  This declaration would create a new aspect instance only for
top-level types.  This would match the typical logging policy document.

There's still a catch for this to work.  We have to allow advice from this
aspect bound to the outermost type to run for code within all inner and
anonymous types.  This is what we should define as the default behavior.

Do we also need a way to disable this behavior and have the code run only
for code within exactly the type T that the aspect is bound to?  One
situation where we might want this would be if we wanted to use a DoNotLogMe
tagging interface (or in the future a tagging attribute) to disable logging
for code within a particular type.  This design will only work if the tag is
placed on the outermost type.  If it is placed on an inner type, logging
will still occur.

We could consider adding a perexacttype(T) or pertype(T, exact) form to
handle this case.  However, I believe that our best course for 1.2 is to
just ignore this edge case until we have more experience with pertype.

---------------------------------------------------------------------
Constructor signature

Consider once again the original motivating example for this feature.  The
use of before advice to initialize the log field is a little awkward.  This
kind of pattern will probably become easier to read over time as a
well-known idiom.  However, it's much better Java style to initialize all
instance fields within a types constructor.  We could make this possible
here if we define the constructor signature for a pertype aspect to take a
single Class argument.  This is different from all other kinds of aspects
whose constructors take zero arguments.  This would allow us to replace the
before advice on staticinitialization(com.boxes..*) with:
  Logger(Class loggedClass) {
    log = new Log(loggedClass);
  }

Using this signature for the aspect's constructor has two main benefits:
1. It makes initialization simpler and easier to get right
2. It removes the redundant second specification of com.boxes..*


---------------------------------------------------------------------
Implementation issues

The straight-forward implementation of Option A looks something like this:
class T {
    private static final AspectType aspectInstance = new AspectType();
    
    public void m() {
        if (aspectInstance != null) aspectInstance.beforeAdvice();
        <original body of m>
    }
}

The one surprising part of this implementation is the null test for the
aspectInstance.  This is required even though aspectInstance is a final
field.  The explanation for this is at the end of this message in the
section on clinit weirdness. 

The straight-forward implementation of Option B is more complicated, it
looks something like this:
class T {
    private static AspectType aspectInstance;

    private static synchronized void bindToAspectIfNotAlreadyBound() {
        if (aspectInstance == null) aspectInstance = new AspectType();
    }
    
    static {
        bindToAspectIfNotAlreadyBound();
        <original body of static initializer>
    }
    
    public void m() {
        bindToAspectIfNotAlreadyBound();
        if (aspectInstance != null) aspectInstance.beforeAdvice();
        <original body of m>
    }
}

The call to a synchronized method before every join point that matches
within(T) adds a considerable overhead.  This could be solved in two ways.

1. If the programmer uses pertype(staticinitialization(T1)) the
straight-forward implementation will produce the same performance as Option
A.

2. However, it's very likely that programmers will often either directly
write pertype(within(T1)) or they will write something equivalent to that
using indirect references through pointcuts.  We need to decide how much
effort should be spent optimizing for these cases.  Note that even the
apparently trival case of within(T1) is made challenging due to the strange
<clinit> issues discussed below.

----------------------------------------------------------
Clinit Weirdness

How could code run that's within(T1) before T1's staticintialization has
even started?  This is a weird corner case of Java's initialization rules.
This same issue is responsible for a bug with references to
thisJoinPointStaticPart that occur in this same weird context.  Here's an
example program:

public class Main {
	public static void main(String[] args) {		
		System.err.println("Derived.getName() = " +
Derived.getName());
		System.err.println("Base.baseName = " + Base.baseName);
	}
}

class Base {
	static Object baseName = Derived.getName(); //!!! this call is
trouble
}

class Derived extends Base {
	static final Object name = "Derived";
	static Object getName() {
		System.err.println("name = " + name);
		Thread.dumpStack();
		return name;
	}
}

If you run this program, you'll find that the first thing it prints is "name
= null".  How could this be when name is a final static field initialized to
a non-null value?  The problem is with how the JVM resolves the circular
dependence between the static initializers for Derived and Base.  Most
people believe this is a mistake in the design of the JVM, but for now we're
stuck with it.  The problem for AspectJ is that even though this kind of
circular dependency in initializers is awful code we can never let this
cause a new NullPointerException that wasn't already in the user's code.

-------------------------------------------------
Minor Frustration

I can't use this version of pertype to write a decent abstract version of
the singleton pattern.

abstract aspect SingletonAspect {
    public Object instance;

    protected abstract pointcut creation();
    protected abstract Object makeInstance();

    after() returning: creation() { instance = makeInstance(); }
}

aspect MySingleton pertype(MyClass) {
    protected pointcut creation(): staticinitialization(MyClass);
    protected Object makeInstance() { return new MyClass(); }
}

The singleton is referenced using:
  (MyClass)(MySingleton.aspectOf(MyClass.class).instance)


vs.

aspect MySingleton1 {
    public static Object MyClass.instance = new MyClass();
}

The singleton is referenced using:
    MyClass.instance