Community
Participate
Working Groups
One of the major causes of performance and memory overhead with LTW in typical middleware containers is the need to keep a full weaver in memory for each active loader. This strategy causes a lot of overhead for tiny loaders that load just a single class, notably for JSP's. This may also be an issue for some implementations of middleware proxies (though the current implementation avoids some cases for RMI and for reflection delegates). A good mid-term optimization for both would be to allow custom weaving contexts to handle specialized loaders like JSP loaders in a way that shares weaving state and never caches types. However, one short-term option would be to allow evicting adaptors that haven't been used for some threshold time (e.g., in the last 1 minute or 5 minutes). This could allow a major reduction in overhead in a typical server with hundreds of JSP's (e.g., Glassbox uses about 1 megabyte of memory per loader for AspectJ weaver state). I believe this would be a safe operation: if the weaver had to be reloaded it should work correctly but it would involve the usual tradeoff of increased time overhead for reduced memory use. One counter argument to this optimization is the likelihood that the memory used by an inactive weaver should be out of the working set, once it has migrated to an older generation space: it won't be collected and will be visited only rarely when checking for GC. However, this counter argument would need to be verified with some benchmarks, and also doesn't address the issue that some systems hit a limit on maximum addressable heap size (e.g., I've found it hard to increase heap to 1.5 gigs running Java on Windows XP even with 4 gigs of contiguous disk for virtual memory). This could be a option could be set through a -D flag or (if a persistent system configuration mechanism were implemented) the JMX interface. I'd be happy to create a patch if there's a consensus that this idea is worth pursuing.
An alternative approach that's is esaier to manage would be keeping references to weaving adaptors. Conceptually, the information in one is just a cache because it can be recreated at any time. I did a benchmark using AspectJ head vs. the attached patch that uses only soft references. As expected, this allows much lower overhead: in this test I started Tomcat with Glassbox, and 2 open source sample apps and I ran a load test script that hit several pages of one of the apps, resulting in the creation of 17 worlds. The CVS head version used 36 mb of heap of which AspectJ used 14 mb. The reference version used 23 mb of heap of which AspectJ used 1.5 mb (org.aspectj.weaver.Dump has a hard reference to the most recently used world, which is probably helpful by avoiding evicting the most recently used weaver). I don't believe the change imposed significant overhead (it spent no more time initializing weavers and though it ran 10% slower in weaving I believe this was an artifact of the randomness of how load test requests came in). However, careful benchmarking would be required to determine how frequent cache evictions might be. I ran some of the pre-commit tests with the attached patch and found 2-3 test failures that were caused by duplicate generation of classes (which is clearly a possibility if a loader is evicted and then on recreation the aspects are again woven and re-registered). The defineClass method already notes there are cases where generated classes can pre-exist, although it would be better to resolve this issue by testing for existing in advance rather than relying on exceptions. However, the duplicate definition is handled and should be harmless. To implement an approach like this we could use a variety of options: hard references could be the default (preserving existing behavior), and we could allow soft or even weak references. It would also be possible to use soft references but to keep some number of hard references to recently used weavers. In any event, having some option would be good for those who need to further limit memory use.
Created attachment 51237 [details] loadtime module patch of prototype implementation (doesn't allow configuration options, just uses soft references)
There are several issues to be addressed here: 1. It has not yet been established that the state of the World/Weaver can be recreated. This has already been discussed in Bug 148773 “Major LTW Optimization: Let Loaded Types be Expendable”. 2. A 10% increase in weaving time is an expensive trade-off. 3. We cannot simply ignore duplicate Closure classes. Their existence has been evidence of bugs in the past. It also suggests that we cannot safely throw away the weaver state. 4. Recreating the adaptor from scratch involves reloading aop.xml configurations that may have changed on disk. This may be unlikely but could be the cause of confusing bugs. 5. Sharing mutable weaving state has been suggested and rejected before because it makes assumptions about class loader relationships that cannot be made e.g. OSGi, non-delegating Web Application class loaders. Additionally we cannot rely on user provided configuration to guarantee the correctness of the weaver. A reduction in Weaver/World footprint relies on the ability to throw away state that is either no longer required or can be accurately recreated. Currently we can throw away types that are used solely for resolution. One thing we could consider is extending the policy to types that are only subject to dynamic crosscutting i.e. no schema changes or that are not woven at all. This, I hope, would cover the majority of woven types: we would still need to hold on to aspects and types with ITDs. Do you think this assumption holds in your environment?
(In reply to comment #3) > There are several issues to be addressed here: > 1. It has not yet been established that the state of the World/Weaver can > be recreated. This has already been discussed in Bug 148773 “Major LTW > Optimization: Let Loaded Types be Expendable”. Clearly a major reason for concern is the current limitation in load-time weaving that completing binary types seeks to address. The load-time weaver currently produces inconsistent and incorrect results depending on load order (bug #133770). So I'll agree that this enhancement request also depends on that. While handling #133770 is unpleasant, I think it has to be resolved. When it is solved, then what other possible problems could arise from recreating state? Inconsistent naming or closures (as in #3 below)? > 2. A 10% increase in weaving time is an expensive trade-off. If that's representative it is somewhat expensive, but it's still worth it for some users who'd like to reduce footprint. > 3. We cannot simply ignore duplicate Closure classes. Their existence has > been evidence of bugs in the past. It also suggests that we cannot safely throw > away the weaver state. If Closures are truly duplicates, then their existence should be safe, right? But the weaver would need to generate consistent names regardless of load/weave order to allow throwing away weaver state (i.e., you can't generate an inconsistent version with references to different types). This might well cause problems with the cflow munger naming. What kinds of bugs have duplicate Closures indicated? > 4. Recreating the adaptor from scratch involves reloading aop.xml > configurations that may have changed on disk. This may be unlikely but could be > the cause of confusing bugs. I would argue that this is akin to the bugs you get if you start replacing jar files on the classpath of a server. If you change configuration in mid-flight it can result in problems. If this were important to avoid, then I'd support caching aop.xml files (hopefully parsed) by URL, which would also be a good (unrelated) optimization. > 5. Sharing mutable weaving state has been suggested and rejected before > because it makes assumptions about class loader relationships that cannot be > made e.g. OSGi, non-delegating Web Application class loaders. Additionally we > cannot rely on user provided configuration to guarantee the correctness of the > weaver. Sharing weaving state certainly isn't trivial, but my view is that it's the right long-term direction. To handle this requirement, I think setting up weaving contexts based on recognized class loaders would allow for correct behavior. > A reduction in Weaver/World footprint relies on the ability to throw away state > that is either no longer required or can be accurately recreated. Currently we > can throw away types that are used solely for resolution. One thing we could > consider is extending the policy to types that are only subject to dynamic > crosscutting i.e. no schema changes or that are not woven at all. This, I hope, > would cover the majority of woven types: we would still need to hold on to > aspects and types with ITDs. Do you think this assumption holds in your > environment? I've already been using the evict reweavable types optimization in normal use (not for these comparisons, which did work from HEAD). Indeed, my types are subject only to dynamic crosscutting (except for some odd privileged access mungers on Object.* and String.* that I don't understand and that don't appear to change the types). So I would certainly support implementing #148773 in cases where the woven type was subject to dynamic crosscutting, even before #133770 is resolved. However, even with this greatly improved memory usage, I still see the 1-1.5 megs of weaver overhead per class loader, which becomes problematic as you increase the number of loaders often to 100's on a given VM. Evicting them is clearly worse than sharing state because the other problem they impose is noticable latency (e.g., adding 100's of milliseconds when loading a JSP).
I must say that I dislike bug reports, whether they are problems or enhancements, which propose a very specific solution. They lead to a lengthy discussion about the appropriateness of that solution rather than the validity of the problem or value of the enhancement. They also make it difficult for less experienced users browsing the database to judge whether they are encountering the same bug or would benefit from a suggested feature. Raisers are of course encouraged to submit patches but it is the responsibility of the committer to implement, test and maintain the code in a variety of environments. The “big bang” solution attached must either be accepted or rejected and given the number of drawbacks identified as well as dependencies on solutions to other bugs, including Bug 133770 “[ltw] Problem with Declare Parents and Call Munger in Load-Time Weaving” which has no obvious solution, I am inclined towards the latter choice. So this is where I make my point: I am not averse to reducing memory footprint just the solution you are proposing. The weaving adaptor _must_ have the same lifetime as the loader with which it is associated especially it we are to add JMX support otherwise it will just be confusing. Instead we must concentrate on releasing or reducing the long term (recoverable) footprint of ResolvedType instances.
Hi Matthew, I appreciate the value of separating requirements from design. I like to use bugzilla to propose possible designs for patches I'm working on. Of course I'd be even happier if the AspectJ team would just optimize load-time weaving to meet our needs but I believe I can help more by contributing. I am also thinking that it might be better for me to request more pluggability for embedded use so we can use AspectJ in our own way without exposing potentially confusing options to users. We'd then add some of our own subclasses and package it as something other than aspectjweaver.jar, and we'd, of course, own support. Anyhow, I'll make an extra effort to start with issues that refer to the requirement/desired feature rather than jumping into a design, point well taken.
