Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClassUtils.getInterfaceMethodIfPossible overhead in cached methods for SpEL key/condition expressions #24206

Closed
tawek opened this issue Dec 13, 2019 · 1 comment
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Milestone

Comments

@tawek
Copy link

tawek commented Dec 13, 2019

Affects: 5.1.0+

Since 'ReflectiveMethodExecutor invokes interface method if possible' (c4df335) has been committed there is a new method getInterfaceMethodIfPossible.
This method unfortunately triggers exception in reflection code which is being catched and ignored as many times as there are interfaces on a class implementing the method (and all of its superclasses), which has an impact on performance (NoSuchMethodException is not pre-allocated so it is not optimized by OmitStackTraceInFastThrow JVM option) especially when such a method is not found (all interfaces have to be traversed).

The issue has been observed in repository code that uses @Cacheable and @CachePut annotations that use SpEL to select keys and describe conditions under which objects are cached.
Spring cache infrastructure creates a new SpEL evaluation context for invocation of cached method when key or condition expressions are specified and populates that new SpEL context with new instances of reflection accessors. The accessors then will trigger ClassUtils.getInterfaceMethodIfPossible in canRead and then cache the result. However since the new SpEL context is created for each method invocation this is caching ineffective.

This will affect probably any SpEL expression frequently executed in the system that is run with a new evaluation context for each invocation.

One possible remedy is to introduce cache on ClassUtils.getInterfaceMethodIfPossible (which I did) or try to fix the spring caching infrastructure (more complex).

I'm sharing the repository with sample code showing the effect here : https://github.com/tawek/classutils-interface-methods-issue

On my HW the overhead of calling @Cacheable method with issue mentioned above is about 127us (~ 7800 calls per second).
When cache is introduced into ClassUtils.getInterfaceMethodIfPossible to avoid exception throwing the overhead is reduced to 10us (~91 000 calls per second). The measurements where done on OpenJDK8 ( OpenJDK11 gives similar results).

The issue is visible if SpEL expressions are more complex and use property access or method calls and if there are more interfaces on navigated objects. The workaround is not to use SpEL expressions if you want low cache overhead and perform lots of cached calls. :(
Potentially this may hit other heavy users of SpEL since that ClassUtils method is invoked from org.springframework.expression.spel.support.ReflectivePropertyAccessor.canRead and org.springframework.expression.spel.support.ReflectiveMethodExecutor. which are both quite common.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 13, 2019
@jhoeller jhoeller self-assigned this Dec 14, 2019
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 14, 2019
@jhoeller jhoeller added this to the 5.2.3 milestone Dec 14, 2019
@jhoeller jhoeller changed the title CacheUtils.getInterfaceMethodIfPossible introduce overhead in cached methods when SpEL key or condition expressions use accessors and navigate objects with interfaces ClassUtils.getInterfaceMethodIfPossible overhead in cached methods for SpEL key/condition expressions Dec 14, 2019
@jhoeller jhoeller added the for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x label Dec 14, 2019
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Dec 14, 2019
@tawek
Copy link
Author

tawek commented Dec 17, 2019

Looks good. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

3 participants