Re: Re[4]: [aspectj-users] Once again aboutn non-this access

["Ron Bodkin" <rbodkin@xxxxxxxxxxxxxx>]

Hi Konstantin,

I now understand why you want to detect foreign field access. So you really do need to check for whether this == target; the fact that the field is from the same type doesn't help you (because you could load one instance of a type but not another).

One thing that's always good to keep in mind when discussing performance is that modern VM's are _really good_ at optimizing. In general, one should always ask whether AspectJ needs to perform an optimization or whether the VM should do it.

With that in mind, I write a small test that measures performance for 3 cases: 
1) a method that isn't ever advised 
2) a method that is advised with local access (where the pointcut excludes local access)
3) a method that is advised with foreign access (where the pointcut excludes local access)

See the end of this email for the exact code I ran. Using JDK 1.4.2_01 on my Windows XP laptop here is an example run:

C:\devel\test\perf>java DynamicTest 1000000000
not advised
Elapsed time is 21280
local advised
Elapsed time is 21281
non-local advised
Elapsed time is 27680

So a modern VM already implements the optimization you want (in cases where the test result can be resolved statically - *). In other words, implementing an AspectJ optimization wouldn't actually improve performance! Now I can see benefits for implementing the optimization (e.g., more accuracy in identifying where advice runs for tools, smaller bytecode, and even better performance on old VMs). But I think for many uses the current AspectJ support would be helpful.

A couple of other quick notes:
- the getField() pointcut you described works for me. You can write:
aspect Persistence {
  public interface Persistent {}
  declare parents: com.example..impl.* implements Persistent;

  pointcut getField(): get(!transient !static Persistent+ *.*);
  // ...
}
Are you having problems defining Persistent? You can define this as a marker interface and use declare parents to denote what classes are persistent. The only case where I can see this causing problems is if you would like to persist third party classes. Is this why you said this pointcut isn't accepted by AspectJ!?

Test Code:

DynamicTest.java:
import java.util.Date;

public class DynamicTest {
    private int x;
    private static DynamicTest associate = new DynamicTest();
    private boolean isLoaded = true;

    void localIsAdvised() {
        x++;
    }

    void nonLocalIsAdvised() {
        associate.x++;
    }

    void notAdvised() {
        x++;
    }

    public static void main(String args[]) {
	int count = 10000;
	if (args.length == 1) {
	    count = Integer.parseInt(args[0]);
	}

	DynamicTest t = new DynamicTest();
	t.testIt(count);
    }

    private void testIt(int count) {
	System.out.println("not advised");
	repeat(count, new Runnable() { public void run() { notAdvised(); } });
	System.out.println("local advised");
	repeat(count, new Runnable() { public void run() { localIsAdvised(); } });
	System.out.println("non-local advised");
	repeat(count, new Runnable() { public void run() { nonLocalIsAdvised(); } });
    }

    private void repeat(int count, Runnable r) {
        for (int i=0; i<count; i++) {
	    r.run();
	}
    }

    private static aspect CheckFieldAccess {
	pointcut settingField(DynamicTest targ) : 
	    withincode(* *isAdvised()) && get(* DynamicTest.x) && target(targ);

	before(DynamicTest curr, DynamicTest targ) : settingField(targ) && this(curr) && if(curr!=targ) {
	    targ.checkLoaded();
	}

	void DynamicTest.checkLoaded() {
	    if (!isLoaded) {
		// slow operation
	    }
	}
    }

    private static aspect Timing {
	void around() : execution(void DynamicTest.repeat(..)) {
	    Date start = new Date();
	    proceed();
	    Date end = new Date();
	    System.err.println("Elapsed time is "+(end.getTime() - start.getTime()));
	}
    }
}


Ron

* - as Wes notes, you can't always statically determine whether this==target

Ron Bodkin
Chief Technology Officer
New Aspects of Software
m: (415) 509-2895

> ------------Original Message------------
> From: Konstantin Knizhnik <knizhnik@xxxxxxxxx>
> To: Ron Bodkin <aspectj-users@xxxxxxxxxxx>
> Date: Tue, Feb-3-2004 7:35 AM
> Subject: Re[4]: [aspectj-users] Once again aboutn non-this access
> 
> Hello Ron,
> 
> Monday, February 2, 2004, 3:42:08 AM, you wrote:
> 
> RB> Hi Konstantin,
> 
> RB> I'd like to step back to look at your requirements. I gather
> RB> you are trying to implement lazy loading, i.e, to detect object
> RB> instances that haven't been loaded so you can load them on demand.
> RB> Is that right?
> 
> Yes, you are right: object will be loaded on demand.
> 
> RB> If so, then you need a mechanism that checks field accesses
> RB> (and accesses to a member of a collection!) to see whether these
> RB> objects are loaded or not. In general, this has to be a dynamic
> RB> test (with the exception of types that are loaded by value such as
> RB> primitives). Consider:
> 
> RB> public class Tree {
> RB>     private Tree parent;
> RB>     private Collection children;
> RB> ...
> RB> }
> 
> RB> The persistence implementation should allow parent or any
> RB> member of children to be lazily loaded. But both parent and the
> RB> members of children are instances of Tree. So you really need a
> RB> dynamic check for these accesses: the referred to instance might
> RB> not be loaded, even though parent is a field on Tree and Tree is
> RB> loaded. There are several approaches to implementing this kind of
> RB> test (either proxies or dynamic tests), but describing the various
> RB> options and how AspectJ can support them is too much detail for
> RB> this email.
> 
> 
> Certainly dynamic check is always needed to check whether object is
> loaded or not. There is only one case when I do not need to perform
> this checking - and actually it is most commonly used case: access to
> self instance variables. Because in any case object is already
> loaded.
> 
> RB> So I don't see how your original request will solve this
> RB> problem.
> 
> Ok, I will try to briefly explain how lazy object loading was
> implemented for my GOODS database (using my own bytecode preprocessor)
> and for Perst using JAssist. First of all some introduction:
> 
> 1. Each persistent object has object identifier (OID) - integer number
> allowing to uniquely identify object within storage.
> 2. When object is loaded, it is obvious how to assign values to
> scalar and string components of the object. But there are two possible
> ways of representing references to other persistent objects.
> Since Java is safe language with static type checking,
> reference field can refer only to the object which type is the same or
> derived from the type of the field.
> - So we can store OID of referenced
>   object somewhere also and store null in reference field (in this case
>   we trap access to the field and load it on demand).
> - Or we can create stub object which type is the same as real type of
>   referenced object which which fields are not loaded yet.
> 
> Both approaches has their advantages and disadvantages.
> I prefer second approach.
> 3. Persistent object has load() method which checks if object is
> already loaded and id not - loads its fields (I want to remember that
> we use second case)
> 
> 
> Now how to implement transparent persistency with this model:
> 1. At the beginning of each instance method we insert load() method.
> So it will ensure that object is loaded.
> If access to the fields of "foreign" (non-this) objects is not
> allowed (it is assumed that setter/getter methods are used instead),
> then this is all we need. We should not insert any extra code or do
> extra checks - fields of this object can be always accessed without
> any extra overhead.
> 2. If access to foreign object fields is allowed, then before
> each such access we should invoke load() method for target object.
> 3. To be able to detect modified objects, before each assignment to the field
> modify() method should be called (it is also possible to call it after
> update).
> 
> Unfortunately (as I already told), it is not possible to implement
> this scenario with AspectJ - it doesn't allow to distinguish this and
> non-this access. Alternative solution is to be able tot detect access
> to reference fields (so we insert load method for each access to
> reference field). Number of invoked load() method with this approach
> is the same as with my approach. But size of code is larger - because
> in my apporach load() is inserted only in one place - in method body,
> and in this approach - in each access to the method or field.
> Also in this case check for null should be done before invocation of
> laod() method.
> 
> But I also do not know how to implement this approach with AspectJ.
> It requires definition of pointcut like this:
> 
> getField(): get(!transient !static Persistent+ *.*);
> 
> (it means access to all fields referencing persistent objects).
> But such pointcut is not accepted by AspectJ.
> 
> 
> All other approaches requires insertion of AspectJ code before (or
> after) each field access. Since OO program used to access fields very
> frequently (there are even recommendations to avoid use of local
> variables because them makes refactoring more difficult), size of the
> program is significantly increased after such preprocessing and speed
> - significantly decreased. I think that AOP should be not just a toy
> or facility for debugging and adding trace messages to your application,
> but really powerful system, which can provide performance compatible (or
> even better) than alternative solutions. In case of persistence or
> remoting aspects alternative solution is to load object explicitly.
> That is why I think that such optimization is absolutely needed.
> 
> 
> 
> RB> But implementing the request does raise some interesting
> RB> points in their own right. So I'll also address the topics of
> RB> optimization, a pointcut that expresses the test that you said you
> RB> wanted, and how this might be enabled by future versions of the
> RB> AspectJ language.
> 
> RB> 1. I would like to see more optimization of the this and
> RB> target pointcut descriptors. There are several cases where the
> RB> emitted bytecode performs runtime checks that can be proven to be
> RB> true or false in all cases. 
> 
> RB> I'd like to see is an optimization of this(Type) (or
> RB> this(var) where var is of Type), which the compiler can always
> RB> resolve to be true or false at compile-time. This results in code
> RB> that may or may not be optimized away by the VM (JIT). I'd most
> RB> like to see this optimization to improve the accuracy of the tools
> RB> support (which currently identifies too many places as advised). I
> RB> believe AspectJ 1.0.x did have this optimization.
> 
> RB> There are several possible optimizations in emitted bytecode
> RB> that it would be desirable for the AspectJ compiler to implement.
> RB> But I think the team's current emphasis on making the compiler
> RB> reliable, scalable, and fast is the right priority.
> 
> RB> 2. I personally don't think adding special case logic to say
> RB> "access to a foreign field" is common enough to warrant a special
> RB> case language extension, and as I will explain below, what you
> RB> really want can't be expressed without a language extension.
> 
> >>From your clarification, you wanted to identify about access to
> >>fields from a different TYPE and not from a different instance.
> >>Translating into AspectJ (plus a logical quantification):
> 
> RB> for all types T:
> RB> for any supertype S of T:
> RB>   pointcut externalGet(T t) : 
> RB>       this(t) && get(* *.*) && !get(* S.*);
> 
> RB> If AspectJ had my proposed type pattern for a type or its supertypes (-), this
> RB> would reduce to:
> RB> for all types T:
> RB>   pointcut externalGet(T t) : 
> RB>       this(t) && get(* *.*) && !get(* T-.*);
> 
> RB> An alternative version (that would also work for static fields) is:
> RB> for all types T:
> RB>   pointcut externalGet() : 
> RB>       // access to a field from any jp within type T or a parent type...
> RB>       (within(T-) || 
> RB>       // or from any ITD's defined on T or a parent type
> RB>        withincode(* T-.*(..)) || withincode(* T-.new(..))) && 
> RB>       get(* *.*) && !get(* T-.*);
> 
> RB> For implementing object persistence, you probably don't care about static fields...
> 
> RB> It _is_ feasible to write a special case of this pointcut for
> RB> a single type in AspectJ (just plug in the appropriate value of T
> RB> and expand to include T and ALL its supertypes). 
> 
> RB> But AspectJ doesn't currently offer a way to quantify over
> RB> types like this at compile-time. Unfortunately, I don't believe
> RB> even the proposed pertype aspect would help here. 
> 
> RB> I am hoping that when AspectJ supports generics, it will be
> RB> possible to express this kind of pointcut, e.g.,
> RB>   pointcut <T> externalGet(T t) : 
> RB>       this(t) && get(* *.*) && !get(* T-.*);
> RB>   after <T> (T t) returning: externalGet(t) {
> RB>       t.dirty = true;
> RB>   }
> 
> RB> I believe that advice for this pointcut could still be
> RB> implemented as a method through type erasure (i.e., it wouldn't
> RB> require macro expansion), though I don't have a fully developed
> RB> proposal for how AspectJ should handle generics. (as an aside 
> 
> RB> So hopefully AspectJ will grow into solving your stated design by 
> RB> 1) defining generics support that will capture it
> RB> 2) adopting my proposal for using T- for a type and all its supertypes
> RB> 3) possibly optimizing the code for this() tests. But note
> RB> that if we have #1 and #2, optimizing this isn't even necessary
> RB> (using the within version of the pointcut will be optimized!)
> 
> RB> Although I don't see how detecting foreign field access helps
> RB> with persistence or remote object invocation.
> 
> RB> Ron
> 
> RB> Ron Bodkin
> RB> Chief Technology Officer
> RB> New Aspects of Software
> RB> m: (415) 509-2895
> 
> >> ------------Original Message------------
> >> From: Konstantin Knizhnik <knizhnik@xxxxxxxxx>
> >> To: Wes Isberg <aspectj-users@xxxxxxxxxxx>
> >> Date: Sat, Jan-31-2004 10:20 AM
> >> Subject: Re[2]: [aspectj-users] Once again aboutn non-this access
> >> 
> >> Hello Wes,
> >> 
> >> 
> >> Thank you very much for your response.
> >> 
> >> WI> You can use within(), but it doesn't really help.
> >> 
> >> WI> A join point has a signature specifying only types, so
> >> WI> it seems like a dynamic test is needed.
> >> 
> >> With dynamic check such feature has completely no sense.
> >> The only reason of this my request is optimization of preprocessed
> >> code. In good OO programs instance variables are usually accessed only
> >> by self or derived classes. Access to foreign variables is not
> >> recommended (better to use setter/getter methods). In Smalltalk such
> >> access is not even possible! Now lets see what's happen if I want to
> >> implement persistence or remoting aspect. The main idea is
> >> transparently load target object. As far as Java is not
> >> Smalltalk nobody can prevent programmer from accessing instance
> >> variable of other objects. So I insert before access to the field
> >> code performing loading of the object. But for self instance variable
> >> this code is absolutely useless - self object is already loaded.
> >> And as far as access to self instance variable happens most frequently
> >> (in well designed program as I said it is 100%), been not able to
> >> distinguish self and not-self access cause significant performance and
> >> byte code size overhead. Concerning your example, I already mentioned
> >> in my previous mail that exact check (that != this) is not
> >> needed: I just want to avoid generation of extra code for normally
> >> accessed instance variables. It will cause completely no problems if value
> >> of target reference is equal to this.
> >> 
> >> Still wonder why nobody else request this feature before - size of
> >> code and will be significantly decreased and performance - increased
> >> (not extra method call for each field access). I think that is very
> >> important for production applications based on AspectJ.
> >> 
> >> WI> Assuming the join point signature were changed to include
> >> WI> an attribute for the invoking reference indicating
> >> WI> whether it was directly (but explicitly or implicitly)
> >> WI> via "this", consider
> >> 
> >> WI>     class Foo {
> >> WI>         int i;
> >> WI>         Bar o;
> >> WI>         int incBoth(Foo foo) {
> >> WI>            return (i++ + foo.i++); // foo:foo==this?
> >> WI>         }
> >> WI>         int incMe() {
> >> WI>            return incBoth(this);
> >> WI>         }
> >> WI>         int incMeAgain() {
> >> WI>            Foo me  = this;        // not a join point
> >> WI>            return incBoth(me);
> >> WI>         }
> >> WI>         public String toString() {
> >> WI>            Bar p = (o == null ? Bar.CONST : o);
> >> WI>            return p.toString();
> >> WI>         }
> >> WI>     }
> >> 
> >> WI> If it were supported, it would seem arbitary and
> >> WI> would hide the real semantics, which are dynamic.
> >> WI> So to me it looks impractical and more confusing than
> >> WI> helpful.
> >> 
> >> WI> Wes
> >> 
> >> WI> P.S. - As for whether this is the right place or whether
> >> WI> anyone replies to a message, that is all purely consensual.
> >> WI> No one is obliged to answer anyone's email.  There is
> >> WI> a general obligation on the part of the committers to
> >> WI> participate in discussions on aspectj-dev pertaining to
> >> WI> the *development* of the technology, but aspectj-users is
> >> WI> for users to communicate with each other.  If people
> >> WI> require immediate support when writing AspectJ code,
> >> WI> they can solicit professional support here or on
> >> WI> aspectj-dev.  (It's generally considered improper for
> >> WI> those offering support services to solicit people who
> >> WI> post to mailing lists unless the posters explicitly
> >> WI> request professional support.)  Also, emails on
> >> WI> questions that can't be resolved by considering the
> >> WI> documentation tend to spark more interest, as do emails
> >> WI> from posters who in the past have contributed correct
> >> WI> and well- written questions and answers.
> >> 
> >> WI> Konstantin Knizhnik wrote:
> >> 
> >> >> Hello,
> >> >> 
> >> >> Three days ago I have sent the question to this mailing list about
> >> >> possibility to distinguish access to this and foreign
> >> >> objects with AspectJ pointcuts. I received no replies to my message.
> >> >> I wonder if this mailing list is the right place for asking such
> >> >> questions? Its it really checked by Aspect-J developers?
> >> >> 
> >> >> I briefly repeat my message here. To be able to efficiently implement
> >> >> persistence and remoting aspects, it will be very useful to be able to
> >> >> distinguish access to self instance fields and access to fields of
> >> >> other objects (non-this access). In last case, such accesses should be
> >> >> wrapped with advice code which is responsible for object loading.
> >> >> But there is completely no need to insert such code for access to self
> >> >> instance variables. Right now I find only one way how to express
> >> >> correspondent pointcut in AspectJ: comparing result of this() and
> >> >> target(). But it is translated by AspectJ in runtime check! So it is
> >> >> in any case impossible to avoid AOP overhead for accessing self
> >> >> instance fields.
> >> >> 
> >> >> Actually what I want to know
> >> >> 1. Is such feature currently supported by Aspect-J?
> >> >> 2. If not - what is the reason:
> >> >> - nobody requested such feature before
> >> >> - technical problems with implementing this feature (it seems to me
> >> >> that there are completely no problems)
> >> >> - some ideological arguments against adding this feature
> >> >> - this feature will not be used by most of Aspect-J users
> >> >> - performance is not critical for most of Aspect-J users.
> >> >> 3. Also are there plans to implement it in 1.2 version of Aspect-J?
> >> >> 
> >> 
> >> WI> _______________________________________________
> >> WI> aspectj-users mailing list
> >> WI> aspectj-users@xxxxxxxxxxx
> >> WI> http://dev.eclipse.org/mailman/listinfo/aspectj-users
> >> 
> >> 
> >> 
> >> -- 
> >> Best regards,
> >>  Konstantin                            mailto:knizhnik@xxxxxxxxx
> >> 
> >> _______________________________________________
> >> aspectj-users mailing list
> >> aspectj-users@xxxxxxxxxxx
> >> http://dev.eclipse.org/mailman/listinfo/aspectj-users
> >> 
> >> 
> RB> _______________________________________________
> RB> aspectj-users mailing list
> RB> aspectj-users@xxxxxxxxxxx
> RB> http://dev.eclipse.org/mailman/listinfo/aspectj-users
> 
> 
> 
> -- 
> Best regards,
>  Konstantin                            mailto:knizhnik@xxxxxxxxx
> 
> _______________________________________________
> aspectj-users mailing list
> aspectj-users@xxxxxxxxxxx
> http://dev.eclipse.org/mailman/listinfo/aspectj-users
> 
>