Bug 113511 - LTW enhancements
Summary: LTW enhancements
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: DEVELOPMENT   Edit
Hardware: All All
: P2 normal (vote)
Target Milestone: 1.5.0RC1   Edit
Assignee: Alexandre Vasseur CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 112817 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-10-24 05:39 EDT by Alexandre Vasseur CLA
Modified: 2005-11-22 04:33 EST (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandre Vasseur CLA 2005-10-24 05:39:33 EDT
Alex, here is the basic patch that is working well before you leave for the
weekend ;-) I'll let Matthew post it officially to bugzilla since it was his
idea. The earlier return from !enabled is a small addition I made that seems
to help further. It would be great to have this in HEAD so I can report
performance numbers based on it in part 2 of my article on developerworks
;-)

Hope you are enjoying your new addition!

Thanks!

Index: ClassLoaderWeavingAdaptor.java
===================================================================
RCS file:
/home/technology/org.aspectj/modules/loadtime/src/org/aspectj/weaver/loadtim
e/ClassLoaderWeavingAdaptor.java,v
retrieving revision 1.18
diff -u -r1.18 ClassLoaderWeavingAdaptor.java
--- ClassLoaderWeavingAdaptor.java      19 Oct 2005 13:11:36 -0000      1.18
+++ ClassLoaderWeavingAdaptor.java      21 Oct 2005 16:04:41 -0000
@@ -107,8 +107,13 @@

        // register the definitions
        registerDefinitions(weaver, loader);
+        if (!enabled) {
+               return;
+        }
        messageHandler = bcelWorld.getMessageHandler();

+
bcelWorld.setResolutionLoader((ClassLoader)null);//loader.getParent());
+
        // after adding aspects
        weaver.prepareForWeave();
    }
@@ -148,7 +153,11 @@
                           definitions.add(DocumentParser.parse(xml));
                       }
               }
-
+               if (definitions.isEmpty()) {
+                       enabled = false;
+                       return;
+               }
+
            // still go thru if definitions is empty since we will
configure
            // the default message handler in there
            registerOptions(weaver, loader, definitions);
Comment 1 Alexandre Vasseur CLA 2005-10-24 05:48:56 EDT
commited this one (TomcatGlassBox startup 28s -> 21s)

please attach next for World sharing on java.* things
Comment 2 Alexandre Vasseur CLA 2005-10-24 06:31:30 EDT
RC1
Comment 3 Alexandre Vasseur CLA 2005-10-25 11:47:43 EDT
*** Bug 112817 has been marked as a duplicate of this bug. ***
Comment 4 Adrian Colyer CLA 2005-10-28 07:24:00 EDT
using P2 to track things for RC1 (P3 or below will not make the cut...)
Comment 5 Ron Bodkin CLA 2005-11-14 11:47:55 EST
The reflection delegates code is now requiring a special type of world, a 
ReflectionWorld, which causes my previously working optimization that used 
reflection delegates when loading bootstrap classes to fail. This will also 
cause problems with any attempt to use reflection-based proxies to optimize 
instead of using BCEL proxies...

In ReflectionBasedReferenceTypeDelegate this method fails now:
	protected ReflectionWorld getReflectionWorld() {
		return (ReflectionWorld) this.world;
	}

My changed code that supported this is in BcelWorld. I think Matthew's right 
that it is going to be better to replace BCEL delegates with reflection-based 
proxies by weaving into classloaders. However, either way, I think it's going 
to be important to support reflection-based proxies for load-time weaving.

	protected ReferenceTypeDelegate resolveDelegate(ReferenceType ty) {
        String name = ty.getName();
        JavaClass jc = null;
        if (hasResolutionLoader) {
			try {
					String asRes = name.replace
('.', '/').concat(".class");
					// will I resolve this by delegating to 
my parent?
					java.net.URL parentURL = 
parent.getResource(asRes);
					if (parentURL != null && 
parentURL.equals(resolutionLoader.getResource(asRes))) {
						Class resultBoot = Class.forName
(name, false, parent);
						if (resultBoot != null) {
		        			return new 
ReflectionBasedReferenceTypeDelegate(resultBoot, this, ty);			
		
						}
					}
					
					Class resultBoot = Class.forName(name, 
false, null);
					if (resultBoot != null) {
	        			return new 
ReflectionBasedReferenceTypeDelegate(resultBoot, this, ty);			
		
					}
				
			} catch (Throwable e) {
				// proceed with normal
			}
        }
Comment 6 Matthew Webster CLA 2005-11-21 11:01:25 EST
Notes from a conversation with Adrian 10/11/05

We need to reduce startup time and latent foot print. There are 3 things we can do:
1.	Only weave when we need to. We have already identified the “enbled” flag as a way to avoid using a weaver when no aop.xml is present. This could be extended to include situations where no aspects are actually declared or defined.
2.	Use reflection instead of byte-code when it is safe to do so. We can introduce a new LTW World & Weaver which is constructed with a class loader and uses both BCEL/byte-code as well as reflection to resolve types.
3.	Replace byte-code when we are certain that a class has been successfully defined. Woven byte-code must be retained in memory because it is difficult and expensive to recreate. However once a class has been defined the byte-code can be replaced by the Class object. There are 3 levels of optimization possible:
a.	Intercept successful calls to defineClass(), either though custom class loaders or weaving, and call back into the weaver.
b.	Cascade define events from parent loaders for types used in resolution but not defined locally
c.	Use weak references to allow the ResolvedMembers associated with a delegate to be dropped and further reduce latent footprint.

Certain environments make these optimizations difficult. It is not possible to share resources reliably between weaver worlds because the relationship between the class loaders with which they are associated cannot always be determined e.g. OSGi. Using reflection may only be safe when using the boot loader due to non-delegating web application class loaders. Byte-code must continue to be used for aspects as their meta-data is not available through reflection pre-Java 5. However they will represent a small proportion of types in the system and a hybrid delegate could be introduced used where reflection is used for Java meta-data and byte-code for AspectJ meta-data.

Many of these enhancements will be targeted post 1.5.0 therefore I suggest closing this bug as resolved because it addresses the simple enhancement to use the enabled flag. We should raise a separate enhancement for each possible optimization (I have already opened bug 114083 to cover an LTWWorld) to help stage their introduction and wider testing. Using this bug as a long running “bucket” will just be confusing.
Comment 7 Adrian Colyer CLA 2005-11-22 03:27:29 EST
Closing as per matthew's last comment. Ron :- note that the reflection world issue has been fixed under a separate bug report and your optimization should hopefully be working again now.
Comment 8 Ron Bodkin CLA 2005-11-22 04:33:46 EST
I updated bug #114083 with an improved optimization. Unfortunately, it isn't so simple to integrate even just the bootstrap loader performance improvement because it results in ClassCastExceptions from code like this:

	public static ResolvedMember createResolvedMethod(Method aMethod, World inWorld) {
		ReflectionBasedResolvedMemberImpl ret = new ReflectionBasedResolvedMemberImpl(org.aspectj.weaver.Member.METHOD,
				toResolvedType(aMethod.getDeclaringClass(),(ReflectionWorld)inWorld),
				aMethod.getModifiers(),
				toResolvedType(aMethod.getReturnType(),(ReflectionWorld)inWorld),

Sample stack trace (from the debugger the method being resolved is public native int java.lang.Object.hashCode()):

java.lang.ClassCastException: org.aspectj.weaver.ltw.LTWWorld
	at org.aspectj.weaver.reflect.ReflectionBasedReferenceTypeDelegateFactory.createResolvedMethod(ReflectionBasedReferenceTypeDelegateFactory.java:83)
	at org.aspectj.weaver.reflect.ReflectionBasedReferenceTypeDelegateFactory.createResolvedMember(ReflectionBasedReferenceTypeDelegateFactory.java:73)
	at org.aspectj.weaver.reflect.ReflectionBasedReferenceTypeDelegate.getDeclaredMethods(ReflectionBasedReferenceTypeDelegate.java:232)
	at org.aspectj.weaver.ReferenceType.getDeclaredMethods(ReferenceType.java:412)
	at org.aspectj.weaver.ResolvedType.getDeclaredAdvice(ResolvedType.java:682)
	at org.aspectj.weaver.ResolvedType.getDeclaredShadowMungers(ResolvedType.java:719)
	at org.aspectj.weaver.ResolvedType.collectShadowMungers(ResolvedType.java:559)
	at org.aspectj.weaver.ResolvedType.collectCrosscuttingMembers(ResolvedType.java:488)
	at org.aspectj.weaver.CrosscuttingMembersSet.addOrReplaceAspect(CrosscuttingMembersSet.java:58)
	at org.aspectj.weaver.bcel.BcelWeaver.addLibraryAspect(BcelWeaver.java:172)
	at org.aspectj.weaver.loadtime.ClassLoaderWeavingAdaptor.registerAspects(ClassLoaderWeavingAdaptor.java:330)
	at org.aspectj.weaver.loadtime.ClassLoaderWeavingAdaptor.registerDefinitions(ClassLoaderWeavingAdaptor.java:188)
	at org.aspectj.weaver.loadtime.ClassLoaderWeavingAdaptor.initialize(ClassLoaderWeavingAdaptor.java:125)
	at org.aspectj.weaver.loadtime.Aj$ExplicitlyInitializedClassLoaderWeavingAdaptor.initialize(Aj.java:130)
	at org.aspectj.weaver.loadtime.Aj$ExplicitlyInitializedClassLoaderWeavingAdaptor.getWeavingAdaptor(Aj.java:135)
	at org.aspectj.weaver.loadtime.Aj$WeaverContainer.getWeaver(Aj.java:101)
	at org.aspectj.weaver.loadtime.Aj.preProcess(Aj.java:65)