Skip to content

Reuse parsed expression in SpelValueExpressionResolver #46253

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

Closed

Conversation

doumdoum
Copy link
Contributor

@doumdoum doumdoum commented Jul 2, 2025

So far expression is parsed everytime the value is resolved.
My proposal is to reuse the parsed expression.
Since the value resolver is used only for a predefined set of expressions the map should contain only a few items.

@doumdoum doumdoum changed the title Reuse parsed expression Reuse parsed expression in SpelValueExpressionResolver Jul 2, 2025
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 2, 2025
@wilkinsona
Copy link
Member

wilkinsona commented Jul 2, 2025

Since the value resolver is used only for a predefined set of expressions

I don't think we can be certain of that as the ValueExpressionResolver is exposed as a bean. It could then be (mis)used in such a way that the cache becomes very large. Using Spring Framework's ConcurrentReferenceHashMap would avoid this potential problem.

So far expression is parsed everytime the value is resolved. My proposal is to reuse the parsed expression.

I wonder if this may be premature optimisation as we haven't seen any issues with @SpanTag where the SpEL-based resolver's been used for some time now. Do you have performance data showing the expression resolution adding an unacceptably large overhead?

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Jul 2, 2025
@doumdoum
Copy link
Contributor Author

doumdoum commented Jul 3, 2025

@wilkinsona, i did some rudimentary performance tests and the gain is marginal (few micro-seconds).
Maybe not worth it.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 3, 2025
@wilkinsona
Copy link
Member

It's certainly not clear that it's worth it. Let's leave things as they are for now. We can reconsider in the future if it becomes apparent that we need to strike a different balance between memory usage and CPU usage. Thanks anyway.

@wilkinsona wilkinsona closed this Jul 3, 2025
@wilkinsona wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants