Clear AspectJExpressionPointcut cache when it is no longer used

[spring-projects-issues]

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

[spring-projects-issues]

Jon Seymour commented

Correction - the measurements refer to Spring 2.5.6, not Spring 3.0.5.

[spring-projects-issues]

Jon Seymour commented

(With AspectJ 1.6.5)

[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 <aop/> tags) and once with it on. I took a heap dump at the end of each run.

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|<aop/> off|<aop> 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.

[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 <aop/> config parser could add a default definition that results in a PointcutParser being added to the factory. Alternatively, the ApsectJPointcutExpression could lazily register a singleton with the factory if one is not already registered. My guess is that the former approach would be cleaner.

[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.

[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 "<pointcutParser>" and uses that one if it can.

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%).

[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.

[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.

[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.

[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.

[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.

[spring-projects-issues]

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.

[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.

[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?

[sbrannen]

Reopening to investigate whether we can improve something here for Spring Framework 6.1.

[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.

[heowc]

I've tried calling init in the form:

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<String, List<Advisor>> advisorsCache = (Map<String, List<Advisor>>) ReflectionTestUtils.getField(builder, "advisorsCache");
    advisorsCache.values()
                 .stream()
                 .flatMap(Collection::stream)
                 .filter(it -> it instanceof InstantiationModelAwarePointcutAdvisor)
                 .map(it -> (AspectJExpressionPointcut) ReflectionTestUtils.getField(it, "declaredPointcut"))
                 .map(it -> (Map<Method, ShadowMatch>) ReflectionTestUtils.getField(it, "shadowMatchCache"))
                 .forEach(Map::clear);
  }
}

This was pretty useful

[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?

[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. 👍

[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.

[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.

[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.

[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