Skip to content

Use a LinkedHashSet when merging parent bean names - #37269

Open
JunggiKim wants to merge 1 commit into
spring-projects:mainfrom
JunggiKim:simplify-merge-names-with-parent
Open

JunggiKim wants to merge 1 commit into
spring-projects:mainfrom
JunggiKim:simplify-merge-names-with-parent

Conversation

@JunggiKim

Copy link
Copy Markdown

BeanFactoryUtils.mergeNamesWithParent builds its result in an ArrayList and calls contains on it for every name returned by the parent factory. A call that finds no match walks the whole list, so merging m parent names into n local names is O(n*m + m^2) string comparisons.

The list is only ever used as an ordered set: names are appended, never read by index, and contains only rejects duplicates. beansOfTypeIncludingAncestors in the same class already applies the same shadowing rule with a LinkedHashMap. Declaring merged as a LinkedHashSet does the same thing here and makes the membership check a hash lookup.

Only the declaration and the copy that follows it change; StringUtils.toStringArray already has a Collection<String> overload. It has to be a LinkedHashSet rather than a plain HashSet: with a HashSet, AutowiredAnnotationBeanPostProcessorTests.objectProviderInjectionWithNonCandidatesInStream fails.

One behavior change is worth flagging. If a ListableBeanFactory returns the same name twice, the old code keeps both entries and the new code keeps only the first. DefaultListableBeanFactory cannot return a duplicate — beanDefinitionNames and manualSingletonNames are kept disjoint, aliases are skipped, and the FactoryBean branch rewrites a name rather than adding one — and StaticListableBeanFactory iterates a LinkedHashMap. If duplicates from a custom implementation should be preserved, I can leave the ArrayList in place and add a HashSet alongside it for the membership check. The method is only called when the factory has a parent ListableBeanFactory, so nothing changes for a context without one.

I ran the same harness against main and this branch, covering aliases, FactoryBean & names, manually registered singletons and shadowed names. Every returned array matched, in the same order. spring-beans, spring-context and spring-aop all pass check on JDK 25.

BeanFactoryUtils.mergeNamesWithParent() built its result in an ArrayList
and called contains() on it for every name returned by the parent
factory. A call that finds no match walks the whole list, so merging m
parent names into n local names is O(n*m + m^2) string comparisons.

The list is only ever used as an ordered set: names are appended, never
read by index, and contains() only rejects duplicates.
beansOfTypeIncludingAncestors() already applies the same shadowing rule
with a LinkedHashMap. Declaring merged as a LinkedHashSet does the same
thing here and makes the membership check a hash lookup.

The method is only called when the factory has a parent
ListableBeanFactory.

Signed-off-by: Junggi Kim <kimjg2477@gmail.com>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 11, 2026
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Sep 14, 2026
@sbrannen
sbrannen requested a review from jhoeller September 14, 2026 10:39
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: waiting-for-triage An issue we've not yet triaged or decided on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants