Jon Seymour opened SPR-7678 and commented
I have been doing a heap dump analysis of a 64bit JVM that uses Java5, Spring and AspectJ point cuts.
I have observed the following:
- the number of ReflectionWorlds in the JVM is ~= the number of PointCuts declared to Spring (P)
- the number of World.TypeMap entries (T) is proportional to the number of types reachable from declarations of beans that Spring can instantiate (T)
- the number of ReferenceType instances in the JVM is ~= k * the number of World.TypeMap entries
In my case, P is 15, k is observed to be 10 and T is around 4,500.
In other words, the JVM contains 15 * 10 * 5,000 ~ 675,000 ReferenceType objects!
Given that the system only has 4,500 types in it, it seems somewhat absurd that there should be 675,000 ReferenceType objects, but this is what the measurements show.
Is it really necessary that AspectJ creates 10 ReferenceType objects per type?
Is it really necessary that each PointCut introduced by Spring has its own ReflectionWorld?
If these two issues could be fixed, the number of ReferenceType objects consumed by this application could be reduced by a factor of 150 which is not an insignificant saving.
Affects: 3.0.5
Attachments: - issue-7678-2.5.6-0001.patch (9.69 kB) - one-reflection-world-per-factory.patch (10.09 kB) - spring.patch (5.90 kB)
3 votes, 5 watchers
Comment From: spring-projects-issues
Jon Seymour commented
Correction - the measurements refer to Spring 2.5.6, not Spring 3.0.5.
Comment From: spring-projects-issues
Jon Seymour commented
(With AspectJ 1.6.5)
Comment From: spring-projects-issues
Jon Seymour commented
I upgraded AspectJ to 1.6.10-RC1. The number of ReferenceType objects improved somewhat (~30%), however this is only a trifling amount.
I wrote a unit test that does a getBean() on all the beans in our application. I ran it once with the Spring aop support switched off (by removing the \
The application has 1489 beans.
As a result of this analysis, AspectJ puts 8736 (T) distinct entries into maps of type World.TypeMap.
At the end of the run, there are 387,350 ReferenceType instances.
In this case, there are 8 point cuts, and so 8 (P) ReflectionWorld instances.
This allows us to parameterise the model again.
387,350 = k * P * T = k * 8 * 8736 = k * 69888
=> k = 5.54
The value of k ~= 5.6 is less than the value reported previously. I suspect this is largely due to the improvements in 1.6.10-RC1.
The real story is in the simple statistics from the heap dump.
The numbers below are in the format:
quantity|\
off|\ on number of classes|9148|9660 number of objects|881879|6909072 number of object arrays|244903|2846212 number of primitive arrays|118915|1124684 number of instances|1254845|10889628 number of references|2540241|18497746 number of roots|1093|946 number of types|9156|9668 heap usage|52M|523M
In other words, simply enabling 8 point cuts on this application and letting Spring and AspectJ do their thing caused the memory consumption to increase by a factor of 8-10.
I suspect most 8-10x footprint increase could be reduced if each point cut introduced by Spring could share the same AspectJ ReflectionWorld instance since this would allow the AspectJ type analysis to be shared across point cuts.
Comment From: spring-projects-issues
Jon Seymour commented
Here's a sketch of a possible solution...
AspectJPointcutExpression currently builds a PointcutParser on construction.
Instead, construction should be deferred until the parser is first used.
This allows the injected BeanFactory used to obtain the PointcutParser. This bean can be a singleton, so that each AspectJPointcutExpression shares the same parser and hence the same ReflectionWorld instance.
Presumably the \
Comment From: spring-projects-issues
Jon Seymour commented
This is a patch to Spring 2.5.6.SEC01 that allows all the AspectJPointcutExpressions in a bean factory to share the same AspectJ PointcutParser instance and hence same AspectJ ReflectionWorld.
Comment From: spring-projects-issues
Jon Seymour commented
The patch above defers creation of the PointcutParser until it is about to be used, then checks the BeanFactory to see if there is one already registered as a singleton under the name "\
Concurrent attempts to create the singleton are handled by using a retry rather than a locking mechanism so as to avoid creating lock contention issues.
AspectJPointcutExpressions that specify a non-default set of primitives get their own parser (and ReflectionWorld) irrespective of the existence of a singleton.
I didn't get back quite as much memory as I expected, but here are the stats:
Number of classes = 9655 Number of objects = 3071042 Number of object arrays = 1119135 Number of primitive arrays = 541680 Total number of instances = 4741512 Total number of references = 8579008 Number of roots = 1009 Number of types = 9663 Heap usage = 204790328 (204MB)
There is now only one ReflectionWorld instance in the JVM, instead of 7. I have got back ~320MB of the ~471MB I would otherwise have lost (~70%).
Comment From: spring-projects-issues
Jon Seymour commented
Just a note: this patch only deals with Pointcuts declared with XML. The annotation declared point cuts have the same issue and will therefore need a similar solution.
The issue will be slightly more involved in this case because currently there is not a BeanFactory available at the point of construction.
Comment From: spring-projects-issues
Jon Seymour commented
I implemented the above suggestion for annotated pointcuts and tested it with a simple scenario in the live application.
With AspectJ 1.6.5 the heap was 326MB.
With AspectJ 1.6.10.RC1 the heap size was 297MB.
With my patches to Spring (+ 1.6.10.RC1 ) the heap size was 170MB.
( the patch for the annotation declared pointcuts will follow when I get a chance )
jon.
Comment From: spring-projects-issues
Jon Seymour commented
This patch replaces the previous patch (spring.patch).
It includes support for sharing a PointcutParser with annotation declared pointcuts too.
Comment From: spring-projects-issues
Jon Seymour commented
Updated patch for 2.5.6, so that it applies cleanly against the Spring maintenance repo.
Also, fixed a null pointer exception in the retry case.
This patch super-cedes one-reflection-world-per-factory.patch and spring.patch.
A patch that applies against 3.0.x will be delivered shortly.
Comment From: spring-projects-issues
Dave Syer commented
I'm pretty sure this is no longer an issue (there were some optimizations in AspectJ 1.8.14ish and also in Spring 5.0.x). I just profiled a simple Spring AOP app using just one pointcut and only found one instance each of ReflectionWorld, ReferenceType and World$TypeMap. With two pointcuts the counts doubled, but the explosion described in this issue doesn't seem to happen any more. If anyone can provide a sample project (I didn't see one in the attachments) I don't mind taking a closer look.
Comment From: spring-projects-issues
Bulk closing outdated, unresolved issues. Please, reopen if still relevant.
Comment From: dmak
I have discovered that AspectJExpressionPointcut holds thousands of entries in field shadowMatchCache. The given cache is not capped, so potentially (however highly unlikely) can cause OOM during startup. The possible improvement could be that AspectJExpressionPointcut listens on context events e.g. is ApplicationListener and empties the cache once the context is started.
Comment From: heowc
I'm writing a spring integration test, and during this process, as @dmak said, I could see that the shadowMatchCache is getting very large. Is there a fundamental solution? Or is there a way to initialize it in tests?
Comment From: sbrannen
Reopening to investigate whether we can improve something here for Spring Framework 6.1.
Comment From: sbrannen
Or is there a way to initialize it in tests?
There is currently no API for resetting or clearing that cache.
However, if you have access to the instance of AspectJExpressionPointcut in question, you could use reflection to access the shadowMatchCache field and invoke clear() on it.
Comment From: heowc
I've tried calling init in the form:
```java
public class ShadowMatchCacheCleanupExtension implements AfterAllCallback {
@Override
public void afterAll(ExtensionContext context) {
final ApplicationContext applicationContext = SpringExtension.getApplicationContext(context);
final AnnotationAwareAspectJAutoProxyCreator creator = applicationContext.getBean(AnnotationAwareAspectJAutoProxyCreator.class);
final BeanFactoryAspectJAdvisorsBuilder builder = (BeanFactoryAspectJAdvisorsBuilder) ReflectionTestUtils.getField(creator, "aspectJAdvisorsBuilder");
final Map
This was pretty useful
Comment From: dmak
There is currently no API for resetting or clearing that cache.
Would it make more sense to reset the cache after Spring context is loaded? Or it is also used after all advisors are wired?
Comment From: sbrannen
This was pretty useful
@heowc, yes, I imagine that proved rather useful.
Kudos for figuring out how to jump through all of the reflection hoops to achieve that. 👍
Comment From: sbrannen
Would it make more sense to reset the cache after Spring context is loaded? Or it is also used after all advisors are wired?
@dmak, those are precisely some of the topics we will investigate in the 6.1 time frame.
Comment From: snicoll
Based on the input of @heowc, I did look at making this available in one form or the other. This turns out to be quite complicated as there are several layers of indirection before hitting the instance that has the cache to clear. There is also a consideration of actually clearing the cache at the right time.
All this point in the direction of a first class concept in the ApplicationContext, available in spring-beans (since Spring AOP does not depend on the context). It could be an interface (name TBD) that a bean can implement to opt-in for being able to clear its cache once the context has refreshed. Ideally such interface would not be tied to the concept of bean so that non-beans can easily opt-in for it as well. Typically, BeanFactoryAspectJAdvisorsBuilder would implement it and cascade on its advisorsCache. And ditto for the Advisor that has a reference to a pointcut that needs its cache cleared.
Another reason for this to be a first class concept is that we may want to actually disable clearing the cache. I can see how Spring Boot Devtools would disable it to prevent advisors to be recomputed over and over again when the context restart.
Perhaps this could be a generalization of AbstractApplicationContext#clearResourceCaches that's currently invoked in finishRefresh?
paging @jhoeller for feedback.
Comment From: snicoll
Another reason for this to be a first class concept is that we may want to actually disable clearing the cache. I can see how Spring Boot Devtools would disable it to prevent advisors to be recomputed over and over again when the context restart.
This was a bogus analysis as the close of the context actually discards the cache anyway.
We've brainstormed and it would nice if we could get away without a first class concept to it. We're considering to rather move AspectJExpressionPointcut's cache to a more central place that we can easily clear, and/or possibly make it static (additionally keyed by the expression) so that it can be cleared from anywhere.
Comment From: snicoll
The cache has been rationalized in ShadowMatchUtils and is cleared automatically once the context has refreshed and on shutdown.
@heowc you should be able to remove that code there when upgrading to Spring Framework 6.2