Use a LinkedHashSet when merging parent bean names - #37269
Open
JunggiKim wants to merge 1 commit into
Open
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BeanFactoryUtils.mergeNamesWithParentbuilds its result in anArrayListand callscontainson 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 isO(n*m + m^2)string comparisons.The list is only ever used as an ordered set: names are appended, never read by index, and
containsonly rejects duplicates.beansOfTypeIncludingAncestorsin the same class already applies the same shadowing rule with aLinkedHashMap. Declaringmergedas aLinkedHashSetdoes the same thing here and makes the membership check a hash lookup.Only the declaration and the copy that follows it change;
StringUtils.toStringArrayalready has aCollection<String>overload. It has to be aLinkedHashSetrather than a plainHashSet: with aHashSet,AutowiredAnnotationBeanPostProcessorTests.objectProviderInjectionWithNonCandidatesInStreamfails.One behavior change is worth flagging. If a
ListableBeanFactoryreturns the same name twice, the old code keeps both entries and the new code keeps only the first.DefaultListableBeanFactorycannot return a duplicate —beanDefinitionNamesandmanualSingletonNamesare kept disjoint, aliases are skipped, and theFactoryBeanbranch rewrites a name rather than adding one — andStaticListableBeanFactoryiterates aLinkedHashMap. If duplicates from a custom implementation should be preserved, I can leave theArrayListin place and add aHashSetalongside it for the membership check. The method is only called when the factory has a parentListableBeanFactory, so nothing changes for a context without one.I ran the same harness against
mainand this branch, covering aliases,FactoryBean&names, manually registered singletons and shadowed names. Every returned array matched, in the same order.spring-beans,spring-contextandspring-aopall passcheckon JDK 25.