Skip to content

Abort early in PropertyPlaceholderHelper if no placeholder exists #22870

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

Conversation

dreis2211
Copy link
Contributor

Hi,

I am currently working on spring-projects/spring-boot#16401 and investigate performance issues in the binding context. I noticed a small optimization opportunity in PropertyPlaceholderHelper if no placeholders exist. We can early out in those cases and thus save the allocations coming from the StringBuilder (and the HashSet).
To be fair: this is not a major performance bottleneck at all (there are ~5000 StringBuilder/HashSet allocations that we could save), but every little helps and who knows what other people might be doing with that class.

Cheers,
Christoph

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 2, 2019
@jhoeller
Copy link
Contributor

jhoeller commented May 4, 2019

I've addressed this slightly differently, only doing the contains/indexOf check once in parseStringValue and rather passing in null for the HashSet, lazily initializing it within the recursion.

Thanks for raising this, in any case!

@jhoeller jhoeller self-assigned this May 4, 2019
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 4, 2019
@jhoeller jhoeller added this to the 5.1.7 milestone May 4, 2019
@jhoeller jhoeller closed this in 9674ce4 May 4, 2019
@sbrannen sbrannen changed the title Early out in PropertyPlaceholderHelper if no placeholder exists Abort early in PropertyPlaceholderHelper if no placeholder exists May 13, 2019
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) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants