Community
Participate
Working Groups
This is a small change to the load-time weaving world that allows evicting non-aspect types (i.e., makes them expendable). This significantly reduces overhead if you weave into a lot of types. This is very different than for compilation, where anything woven has to be kept so it can be dumped out at the end. For load-time weaving, once the type has been defined it is valid to evict it. To test that this is valid I integration tested with Glassbox (we have been using an experimental AspectJ weaver that uses this and other optimization techniques and didn't have problems with it). The 1.5.2 rc1 release has about 160% memory overhead on startup in a test configuration of Tomcat with Glassbox without weaving. With this patch, the overhead drops to 90%, i.e., it almost cuts overhead in half. I wrote a unit test that forces the eviction of a type then reloads it and uses the definition to correctly resolve a call pointcut. I also tested by running load-time weaving tests with an instrumented version of BcelWeaver that calls System.gc frequently during the weaving process to make sure that it doesn't break. This patch couldn't be submitted until recently, when LTWWorld was finally integrated into the codebase...
Created attachment 45359 [details] Change isExpendable logic and register when loading aspects for the LTWWorld
Created attachment 45360 [details] loadtime module patch: call setLoadingAspects on start and end of registering aspect definitions
Created attachment 45363 [details] tests module patch: unit test to verify proper weaving after reloading evicted woven class when load-time weaving
Created attachment 45364 [details] Test only patch to weaver module. NOT for committing to production use, only to be tried and backed out. I am running RunTheseBeforeYouCommitTests with this (slow!) patch that forces GC at critical points during the weaving process. This could be a good virtual mock object for occasional regression testing but it makes the system very slow.
What is the definition of an "expendable" type? To me it is one that we can easily recover at a later time. This enhancement seems to suggest that any type that isn't an aspect or primitive is expendable as long as it wasn't needed to resolve an aspect during weaver initialization. However in a long running system we may want to GC these types if we have reached a steady state wrt weaving. On the other hand LTWWorld.isExpendable() overrides the implementation in World that ensures types exposed to the weaver are not lost. These cannot be recovered because an ITD may change their shape. Whether a type is expendable or not should depend on where we got it not when we got it. So where do we get the byte-code for Advised? From disk. But that won’t have the ITD so why does the testcase pass? Because the woven join point “Woven.failMethod()” does not depend on the new shape of Advised. So can I write a test case that would depend on the new shape? I don’t know. What I do know is it’s way too late in 1.5.2 to override World.isExpendable() especially as the reloaded BcelObjectType has isExposedToWeaver() == false which could have other side effects.
I agree that a type is expendable if it's one we can easily recover at a later time. I believe that for LTW that that should apply to the reference types for anything but the aspect definitions (minimally). If we've evicted the reference types then when we reload it, it should be woven properly. However, your issue about not applying an ITD to an evicted type is an important one: I believe that this one is related to bug #133770 (which is a bug caused by NOT correctly applying the ITD to a BCEL type that is loaded during weaving instead of being woven). In general, I believe LTW has to weave types that are loaded through delegates consistently with how it would weave them when they are loaded through the ClassLoader. I submit that this is a much better design than caching ReferenceType's for each of them. Let's pick this back up after 1.5.2, with a goal of further optimizing memory use in 1.5.3.
I just tested the pipelined compilation changes and see essentially no change in memory use for load-time weaving (not a big surprise: LTW is essentially already pipelined). I also compared the memory overhead of using AspectJ 1.5.2 versus a simpler version of the change for this optimization (just making aspects and primitives not expendible) to CVS HEAD when loading Glassbox on Weblogic. Just to get AspectJ 1.5.2 to load in a gig of memory we had to add these aop.xml exclusions: <exclude within="weblogic..* AND !weblogic.jdbc..* AND !weblogic.j2ee..* AND !weblogic.servlet..*"/> <exclude within="com.bea..*"/> <exclude within="javelin..*"/> <exclude within="com.rsa..*"/> <exclude within="com.certicom..*"/> Even with that, the unpatched version of AspectJ consumes 163 million bytes of overhead on startup. By contrast, the patched version consumes 40 million. I'll attach an HTML report of memory use in each case. With this patch, the main overhead comes from all the redundant copies of weaver state (23 of them holding almost 2 megabytes each). I'd really like us to fix bug #133770 so this optimization can be safely tested and included. As this example illustrates, holding a copy of BCEL for every woven class is a massive memory drain that makes load-time weaving hard to use in large systems.
Created attachment 46965 [details] Memory use report from profiler with optimization
Created attachment 46967 [details] Memory use report from profiler without optimization
I'm thinking the aggressive eviction has to be triggered only if the completion of binary type bindings has been activated (and a correct policy for the classloader system in use has been defined, ie. isLocallyDefined is written correctly) or there are no ITD (or ITD like) constructs coming from the aspects involved in what this particular weaver is doing.
If "eviction" actually means "never storing the information in the first place because we don't need it" (as could be the case for certain woven types) then I agree. However manually freeing a cache at some arbitrary point not knowing whether the information will be needed 2 seconds later doesn't make sense to me. Better to work with GC using Soft/Weak maps so we make use of all the heap we have and only free stuff when space is actually needed.
The most recent version of a patch for this optimization does exactly what you are suggesting Matthew: instead of manually evicting anything it just marks the BCEL for woven types as "expendable", using the World's expendable type map. Items in that map use soft or weak references so they aren't released until a GC claims them. I've been testing with an even simpler version of the patch to LTWWorld that doesn't even track whether we're loading aspects: + protected boolean isExpendable(ResolvedType type) { + return ((type != null) && !type.isAspect() && (!type.isPrimitiveType())); + } This is with no changes to loadtime. This works because the only types we can't recover data from are aspects (which would probably be pinned by the weaver's references to mungers anyhow), dramatically lowers required memory use, and uses references to get good cache behavior. To Andy's point, letting woven types be expendable only when completing binary types or not having ITD or ITD-like mungers sounds right. However, I'd note that the load-time weaver is already buggy when this isn't true, e.g., I've been able to get stack traces using ITD's with LTW even without call pointcuts or the like.
Created attachment 48489 [details] Simplified version of the patch
