Bug 48082 - Support for pertype aspect instantiation model
Summary: Support for pertype aspect instantiation model
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.1.1   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 1.5.0 M2   Edit
Assignee: Adrian Colyer CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-12-04 11:31 EST by Adrian Colyer CLA
Modified: 2005-03-22 08:34 EST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Colyer CLA 2003-12-04 11:31:13 EST
Provide support for a pertype aspect instantiation model - this is needed (as 
documented in the 1.1 release notes) to support designs that require state to be 
held on a per-type basis (for example, some common logging idioms).

Some language design work needs to be done to consider the form of pertype. It 
could be

a) aspect Foo pertype(TypePattern)
this is easy to understand and implement, but at odds with the other perXXX 
clauses that take a pointcut.

b) aspect Foo pertype(<pointcut expression>)
harder to reason about the meaning of "the type(s) at a join point" (types are 
essentially a static thing imho), and harder to implement, but consistent with 
the other perxxx models, and supporting composition and abstraction.

c) as (b), but limit the pointcut expression to only the within pcd (or 
expressions containing only the within pcd). Let the meaning be "types defined 
within(...)". Easy to understand, supports composition and abstraction, but more 
limited than b) and also gives 'within' a different semantic when used in this 
context which is the least pleasant 'aspect' of this alternative

d) something else?
Comment 1 Jim Hugunin CLA 2004-01-30 16:15:38 EST
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
Comment 2 Adrian Colyer CLA 2005-03-22 08:34:33 EST
Fixed in AspectJ 5 M2 with support for the pertypewithin instantiation model.
(see the AJDK Developer's Notebook for details).