getters/setter vs. field interception

[bill.burke]

I've been talking to Bob Lee about some strategies we're taking. He's arguing for getters/setters instead of field interception. If you only allow interception of PUBLIC methods you can be guaranteed as an app developer that no interception will be be done for actions done within the class. This can simplify code for the app developer and for JBoss itself.

BUT....It would be cool to intercept fields :)

[talipozturk]

>only allow interception of PUBLIC methods you can be guaranteed as an app developer that no interception will be be done for actions done within the class. This can simplify code for the app developer and for JBoss itself.

the argument itself is right but it should be up to developers to design, configure things that way. aop framework shouldn't force us to intercept only public methods or not intercept private fields. I think framework should support private public field and method interceptions by default. for developers who doesn't like to intercept private variables or none-public methods, just to make configuration easy for them, jboss-aop framework may want to support a configuration something like

<global-filter-definitions variable-interception="false" publicMethodsOnly="true" />

once again, i think it would be good design decision and good aop practise to intercept with getter/setter methods instead of field, but it should not be enforced by the aop framework.

-talip

[belaban]

I have the following use case. I have a cache (subclass of java.util.Map) into which I put keys and values (which all have to be serializable). Let's take an example:

Cache c;
c.put("name", "Bela Ban");

Whenever I put a new entry, it will be replicated to all cache instances. Caches can be in different processes, on different machines etc.

Now consider a class Person with attrs name and age that I put into the cache:

Person p=new Person("Bela Ban, 37);
cache.put("bela", p);

Now the entry will be replicated to all instances of the replicated cache. But when someone does

p.setAge(p.getAge()++);

the new value of p's age will not be replicated unless we put the item back into the cache:

c.put("bela", p);

See my problem ? If people forget to invoke the put() method, the values in the caches are going to become inconsistent.

Now one could argue that this it is the programmer's fault, but it would be nice to take care of these things automatically.

Another nice aspect would be that we would know exactly which field have changed, and therefore only replicated the field which actually changed, *not* the entire object. We currently replicate the entire object in HTTPSession replication and in clustering (stateful session beans).

If we can satisfy these 2 requirements without private/protected field interceptors, fine. Otherwise I'd make a case for those interceptors (pointcuts).

Bela

[marc.fleury]

> I've been talking to Bob Lee about some strategies
> we're taking. He's arguing for getters/setters
> instead of field interception. If you only allow
> interception of PUBLIC methods you can be guaranteed
> as an app developer that no interception will be be
> done for actions done within the class.


I thought we could do this.field = value; in interception. Am I wrong?

I don't like the point of 'no interception within the class' I WANT interception within the class. The point as Bela Ban points out is that we can then FULLY AUTOMATE THE CACHE and PERSISTENT CACHES (AKA CMP2.0).

> This can
> simplify code for the app developer and for JBoss
> itself.

Negative on the first point. Agreed on the second but irrelevant, that is why we are paying you the big bucks.

Shoot bob lee in the head. I am floating free between the stars, planet e is far-away, I am here to say I am here to stay, floating free, one two three, eternally, no gravity.

[marc.fleury]

> methods or not intercept private fields. I think
> framework should support private public field and
> method interceptions by default. for developers who

yes.

The key is that we can indirect the calls to a stack of interceptors (unlike the hardcoded aspectJ approach) and that this stack is configurable.

What I am thinking is that a class can be treated as a "dataobject" (read XML thingy somewhere mapping this). I am arguing that the default should be no indirection at all (for performance reasons today) but that in the future we should do interception on all.

SO FOR EXAMPLE:
every object that gets loaded gets interception built in, do we do anything with the interception by default? NO we leave the stack absolutely empty and just do the reflective call. This is the main reason why we should keep this turned OFF by default for now as reflection is dog slow. Unless you guys (bob lee?) have some magic trixxs up your sleeves on how to speed up reflective invocation. So all field access looks

o>--<o

where o is the front proxy, > denotes indirection in a invoke() signature for all calls and <o the adaptation to the final object and here is where we want to be as fast as possible (jdk1.3 is 20 times slower than direct invocation so let's TURN IT OFF in the beginning).

But if it is on then let's say we insert the object in a cache with a persistent cache somewhere then what the cache does is add to the interceptor stack (there is a map from hashcode (memory address) to the object stack of interceptors) and the cache can add it's own interceptor thingy so that the object that was inserted in the cache now looks like this

o>-|-<o

where the middle | is an interceptor that keeps track of access to the fields and maybe actual modifications so we can delta mods in cache replication (as bela points out this is a big plus compared to our implementation).

The difference is in what the developer needs to configure. Right now we are going down the road of asking the developer
1- do getters and setters (boblee) which is in fact the CMP 2.0 spec (abstract getters and setters) and this is VERY heavy on the developer as it limits our framework to one semantic of programming. Avoid as much as you can semantic changes as you break portability with existing applications.

2- ask for the developer to specify the stack per scope (bill has that already) this means a slightly static configuration but is XML based and works with existing data. For example we could have a simple XML that says



and that in effect makes sure that we create the indirection in the field accesses and the CMP2.0 like tracking of modified field for cache replication like bela describes and persistent cache SQL storage in a pure CMP 2.0 fashion.

3 Don't require anything from the developer. That means that we instrument the class at least for indirection and set the stack dynamically by aspect. If you want to add the cache aspect, at some point you are going to do an insertion in the cache (lookup, create semantic from the EJB spec) and UPON INSERTION the cache implementation can add its interceptor to the objects that are inserted complete with transitive closure of the fields so we instrument the object for cache replication on the fly.


4 we define whether a class is aspected at deployment time (like we do today) meaning that an object goes from
o to o>-<o but allow for a dynamic access to that stack. BIll I am thinking that this is what we want to do. This allow for speed as we don't pointcut all access to objects and let the object be the native one most of the time but when the flag is on we pointcut. Also there should be a map on o (o.hashcode() is the key) to the stack of interceptors the object uses in the dynamic proxy equivalent. It enables us to do something like the level 3 here without pointcutting everyone in the VM. That says that when the VM's are ready (meaning INTEL is ready with enough moore's law to let us do all this crazy stuff) we can flip the switch and let the VM classloaders pointcut everything. I think we should support a SIMPLE configuration file in the time being something at the xAR level so you can isolate your dataobjects and define that we should pointcut for all classes. Apart from that the current configuration is ok but should allow
, the language for the interceptor filter at the method level is different of course here I am just covering whether we should pointcut or not.


Solution 4 is the future as we will have enought INTEL CPU to pointcut everything like VB does and the reflection price will be negligeable even if present ALL AROUND (on every call). Today this is not the case, JDK1.3 SUCKS DICKS on reflection and unless bob has some magic we are stuck in land 2.


> doesn't like to intercept private variables or
> none-public methods, just to make configuration easy
> for them, jboss-aop framework may want to support a
> configuration something like
>
> <global-filter-definitions
> variable-interception="false"
> publicMethodsOnly="true" />
>
> once again, i think it would be good design decision
> and good aop practise to intercept with getter/setter
> methods instead of field, but it should not be
> enforced by the aop framework.
>
> -talip
>

[marc.fleury]

> Now consider a class Person with attrs name and age
> that I put into the cache:
>
> Person p=new Person("Bela Ban, 37);
> cache.put("bela", p);
>
> Now the entry will be replicated to all instances of
> the replicated cache. But when someone does
>
> p.setAge(p.getAge()++);
>
> the new value of p's age will not be replicated
> unless we put the item back into the cache:
>
> c.put("bela", p);
>
> See my problem ? If people forget to invoke the put()
> method, the values in the caches are going to become
> inconsistent.

This is exactly the problem we have on HTTPSession cache replication. There needs to be an explicit call from the web layers to replicate and when we do we blindly replicate the data across the VMs.

> Another nice aspect would be that we would know
> exactly which field have changed, and therefore only
> replicated the field which actually changed, *not*
> the entire object. We currently replicate the entire
> object in HTTPSession replication and in clustering
> (stateful session beans).

Right. This is precisely where the automated tracking comes in VERY handy.

> If we can satisfy these 2 requirements without
> private/protected field interceptors, fine. Otherwise
> I'd make a case for those interceptors (pointcuts).

absolutely, I am glad the talks we had in SF are not completely lost:). I would add that in fact the pointcut setting is a problem of scope.

See my post above this one where I essentially lay out the scope of pointcutting. Pointcutting is expensive as it involves reflection today, if that can be sped up then we can pointcut everyone and recreate VB (cute). If not then simple XML that would say "this is a dataobject that is going to go in the cache" will create the pointcut and add the interceptor that takes care of tracking field access and maintaining the maps within the invocation/transaction (real CMP2.0 support).

Read my last point (#3) on the fact that in the near future we could get by only defining the pointcut and letting the cache insertion add field interception interceptor at the end of the interceptor stack for the objects inserted in the cache.

Caches on JBoss is going to be a great project.

[bill.burke]

> I have the following use case. I have a cache
> (subclass of java.util.Map) into which I put keys and
> values (which all have to be serializable). Let's
> take an example:
>
> Cache c;
> c.put("name", "Bela Ban");
>
> Whenever I put a new entry, it will be replicated to
> all cache instances. Caches can be in different
> processes, on different machines etc.
>
> Now consider a class Person with attrs name and age
> that I put into the cache:
>
> Person p=new Person("Bela Ban, 37);
> cache.put("bela", p);
>
> Now the entry will be replicated to all instances of
> the replicated cache. But when someone does
>
> p.setAge(p.getAge()++);
>
> the new value of p's age will not be replicated
> unless we put the item back into the cache:
>
> c.put("bela", p);
>
> See my problem ? If people forget to invoke the put()
> method, the values in the caches are going to become
> inconsistent.
>

Don't understand what would be the difference between intercepting methods and interception fields here.

Just do this.

public class CacheInterceptor
{
...
public Object invoke(Invocation invocation) ...
{
Method method = invocation.getMethod();
Object rtn = invocation.invokeNext();
if (method.getName().startsWith("set"))
{
cache.put("bela", targetObject);
}
}
}


> Now one could argue that this it is the programmer's
> fault, but it would be nice to take care of these
> things automatically.
>
> Another nice aspect would be that we would know
> exactly which field have changed, and therefore only
> replicated the field which actually changed, *not*
> the entire object. We currently replicate the entire
> object in HTTPSession replication and in clustering
> (stateful session beans).
>

I think this could be done as long as the app developer follows the getter/setter semantic.

Bill

[marc.fleury]

> Don't understand what would be the difference between
> intercepting methods and interception fields here.
>
> Just do this.
>
> public class CacheInterceptor
> {
> ...
> public Object invoke(Invocation invocation) ...
> {
> Method method = invocation.getMethod();
> Object rtn = invocation.invokeNext();
> if (method.getName().startsWith("set"))
> {
> cache.put("bela", targetObject);
> }
> }
> }

You don't get it.

What he is saying (in fact I think you were the first one to point out the point to me on the HTTPSession thingy) is that once you put the object in the cache, you have no way of knowing that the object is modified. Right now only the "put" operation triggers replication.

What he is talking about is making sure that we replicate object that are already inserted.

if you do

((User) cache.get("Bela")).address = "new address";

or a

((User) cache.get("Bela")).setNewAddress("new address");

which internally does a

public void setNewAdress(String address) {
this.address="new address";
}

in both cases I want that field to be marked for replication and persistence at the end of the transaction. This means that I need to intercept both calls to know that I have touched the field within the transaction.

Are we in sync?

marcf

[bill.burke]

> > methods or not intercept private fields. I think
> > framework should support private public field and
> > method interceptions by default. for developers
> who
>
> yes.
>
> The key is that we can indirect the calls to a stack
> of interceptors (unlike the hardcoded aspectJ
> approach) and that this stack is configurable.
>
> What I am thinking is that a class can be treated as
> a "dataobject" (read XML thingy somewhere mapping
> this). I am arguing that the default should be no
> indirection at all (for performance reasons today)
> but that in the future we should do interception on
> all.
>

Nah, no need. If a field pointcut is defined we instrument the class, if not, then we don't instrument it for field interception.

> SO FOR EXAMPLE:
> every object that gets loaded gets interception built
> in, do we do anything with the interception by
> default? NO we leave the stack absolutely empty and
> just do the reflective call. This is the main reason
> why we should keep this turned OFF by default for now
> as reflection is dog slow. Unless you guys (bob
> lee?) have some magic trixxs up your sleeves on how
> to speed up reflective invocation. So all field
> access looks
>

If I understand the Bob's code correctly, he avoids reflection if there is no interceptor stack. Wonder if we could do the same with fields? I'll have to do some research.

The thing that will be really really tough is public field interception though. This is because to intercepts fields you actually have to instrument the code that accesses the field. And you have to explicitly do this.


Bill

[davidjencks]

>>The thing that will be really really tough is public field interception though. This is because to intercepts fields you actually have to instrument the code that accesses the field. And you have to explicitly do this.

Jdo does this field interception in the enhancement process, and they punt on public fields for this reason: changes in public fields from other classes are not noticed by the jdo persistence mechanism.