Optimize StandardTypeLocator for hotspot when the same classes are resolved

[xuhaihong]

Hi,

We are using spring cache mechanism in the application, something like below

@Cacheable(value = "ProductQueryService.countByParam",
        key = "#param.userId + '#' + #param.category + '#' + T(java.util.Arrays).toString(#param.excludeStatus),
        condition = "T(com.xxxx.CacheConditionUtils).checkIffFieldsExists"
            + "(#param,"
            + " new java.lang.String[] {'userId', 'category','excludeStatus'}")",
        unless = "#result < 10000"
    )

while debugging one performance issue in the production environment, we observed lots of loadClass behaviors due to the using those classes in the condition or key value, which somewhat affects the response time in the concurrent accesses, as the loadClass is marked as synchronized in the classLoader implementation.

It is straightforward that, I am thinking to provide a customized typeLocator, which may avoid the loadClass invocations. After digging the codes for a while, it seems no luck to do that. I could see the following possible enhancements.

1. Remove the final modifier for the evaluator variable in the CacheAspectSupport class, and provide the setter/getter methods to change it, so I could set my own TypeLocator by overriding the createEnavluationContext.
   private final CacheOperationExpressionEvaluator evaluator = new CacheOperationExpressionEvaluator();
2. The inner class CacheOpearationContext in CacheAspectSupport is already marked protected, while the createEvaluationContext method is marked as private, seems that replacing the private with protected may also be helpful.

I am using an old springframework version 4.3.30, but seems that it faces the same issue in the new spring versions.
Thanks !

[snicoll]

@xuhaihong using complex SpEL expressions like these has definitely an impact. You could remove the use of SpEL in your key by a CacheResolver implementation. For the condition, I am surprised to see that repeated calls on the same class leads to a hotspot. What would your TypeLocator look like if you were able to set it?

[xuhaihong]

@snicoll thanks, will take a look at the CacheResolver whether it could help with the key evaluation. For the TypeLocator, I am thinking to maintain a global class cache or possible a singleton customized standardTypeLocator.
The pseudocode codes may look like the fragment below （if considering more comprehensive factors, the cache may be per classloader or using the soft reference.

private Map<String, Class<?>> commonCache = new ConcurrentHashMap<>();

        @Override
        public Class<?> findType(String typeName) throws EvaluationException {
            Class<?> cachedClazz = commonCache.get(typeName);
            if(cachedClazz == null) {
                cachedClazz = super.findType(typeName);
                commonCache.putIfAbsent(typeName, cachedClazz);
            }
            return cachedClazz;
        }

[snicoll]

We've decided to implement that cache out-of-the-box.

[xuhaihong]

@jhoeller @snicoll thanks for coming up this change quickly. IIUC, just with the changes in locator might not work in all the scenairos. It should work in the spring context initialization process, as it is cached in the bean factory, while not in the cache scenario. According to the codes in the CacheOperationExpressionEvaluator.createEvaluationContext, the CacheEvaluationContext is recreated each time, and the typeLocator is not shared among those new created instances, that is why I mentioned in the previous comment that, something like a 'global cache' is required. I am thinking something should be changed on the spring cache implementations. Possibly, it should be allowed to set a shareable typeLocator.

[snicoll]

Good point @xuhaihong! Juergen and I looked at it a bit more and concluded that some more work is needed. Please watch #31617 for updates. We appreciate you taking the time to test and report back.