Optimizing genrated advisors

[kabirkhan]

I've started a stress test framework, which lives under src/test/org/jboss/test/aop/stress. I'll be fleshing it out over the next few weeks and posting my findings here. To run the org.jboss.test.aop.stress.perinstancemethodinvocation test for example

$ build.sh -f build-tests-jdk50.xml main loadtime-ga-test loadtime-test -Dtest=stress/perinstancemethodinvocation

The good news
=============

1) For method interception etc. using per_vm interceptors in generated advisors seem a bit faster than classic despite the extra method call in the weaving model.

2) For method interception etc. using per_vm aspects generated advisors seem about 10% faster than classic, probably because now the generated joinpoint classes call the advice method directly rather than relying on a generated class

3) Executing this 100000 times initially caused an OutOfMemoryError

 <interceptor name="Int1" class="org.jboss.test.aop.stress.perinstancemethodinvocation.PerInstanceInterceptor" scope="PER_INSTANCE"/>

 <bind pointcut="execution(* org.jboss.test.aop.stress.perinstancemethodinvocation.POJO->method1())">
 <interceptor-ref name="Int1"/>
 </bind>


 public void execute(int thread, int loop) throws Exception
 {
 POJO pojo = new POJO();
 pojo.method1();
 }

I have fixed the leak, so this problem does not occur anymore

The bad news
============
* Executing the per_instance example from above with generated advisors takes about 21 times as long as using classic. This is probably because for each new object, (Just working off memory here) if an object has per_instance aspects we then need to obtain a per_instance domain, populate the domain, redo all the bindings, and generate a unique joinpoint class for every single object. So I think we need to differentiate between cases where we actually have overridden something in the instance domain (i.e. dynamic aop) and where we are basically just inheriting everything from the parent.

[kabirkhan]

Holder JIRA task
http://jira.jboss.com/jira/browse/JBAOP-376

[flavia.rainone]

The creation of stress tests are great news for JBoss AOP.

These results are already encouraging, even because they are just initial tests, being run against code that we intend to improve.

Regarding the per_instance issue, I agree with the idea of differentiating domains that haven't overriden the bindings from those that have.
We could store the generated joinpoint classes in a WeakHashMap, where we would map the list of advices/interceptors to the generated class. That way, if:

the joinpoint class is replaced by another one, due to dynamic operations -> the class would eventually be removed from the map
two or more per instance domains (with overriden bindings) happen to have the same collection of advices/interceptors to apply to the same joinpoint -> the needed joinpoint class would be generated only once
two or more instances have a non-overriden domain -> the needed joinpoint class would be generated only once

Plus, after making generated advisors work 100% with hotswap, I suggest that, only in hotswap mode, we do the advices call inside the wrapper instead of a joinpoint class. This way, we would avoid the extra call to the joinpoint class. With hotswap, we will be able of swapping the wrapper code when needed.

[kabirkhan]

Forgot to mention, you can play with things like

* number of warmup loops
* number of test loops
* number of test threads etc

by modifying src/resources/test/stress/config.properties. These properties can also be passed in using the -D option from the command-line

[kabirkhan]

Quite a lot of progress has been made on improving the instantiation times for objects advised by per_instance aspects when using generated advisors. For generated advisors the instance advisor has a per_instance domain, which is populated similarly to the class advisors domain. Main changes for better performance are:

* Make a lot of the maps in AspectManager/Domain only initialise when first accessed. Quite often these are not used, and TLongObjectHashMap and ConcurrentReaderHashMap are very costly to create.
* Make a lot of the maps in Advisor/ClassAdvisor/GeneratedClassAdvisor also only initialise when first accessed for the same reasons as above.
* Clone the JoinPointInfos from the class advisor
* When creating interceptor chains, simply copy/clone the chains from the class advisor if the instance domain contains no bindings/metadata/etc that would cause the instance advisor to have different chains.
* One JoinPointGenerator is maintained per advised joinpoint at class advisor level. There was no need to duplicate these at instance advisor level since the only difference is what is contained in the now passed in JoinPointInfo
* It is not necessary to build instance advisor chains for constructor and construction joinpoints, as the instance already exists
* Field and method tables can be copied from the class advisor, they will be no different in the instance advisor
* There is no need to generate a unique generated invocation class for every instance, if a class has already been generated for an info with the same chains for the same joinpoint

[kabirkhan]

I've improved the configuration of the stress framework. See https://svn.jboss.org/repos/jbossas/projects/aop/trunk/aop/src/test/org/jboss/test/aop/stress/ScenarioPropertyReader.java for available properties. It now consists of several levels, if found at a given level it will use that value, otherwise it will hit the next level. The levels are:

1) System Properties, i.e. -Dwarmup=500
2) Single Test configuration, e.g. src/resources/test/stress/simple/SimpleReflectToJavassistTestCase_testException.properties would configure properties for just that test
3) TestCase configuration, e.g. src/resources/test/stress/simple/SimpleReflectToJavassistTestCase.properties would configure defauls for all tests within that test case
4) Global stress defaults, configured in src/resources/test/stress/config.properties
https://svn.jboss.org/repos/jbossas/projects/aop/trunk/aop/src/resources/test/stress/config.properties
5) Hardcoded default values in DefaultScenarioPropertyReader

[flavia.rainone]

Once when I ran the tests, you told me how important is the warmup property.

What value do you advise me to use on this property, so I that the test results don't be affected by the startup phasis?

[kabirkhan]

The answer AFAIK is that there is no exact answer. Enough iterations for the JVM to run about 10 seconds should do I think