-
Notifications
You must be signed in to change notification settings - Fork 38k
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
Avoid wasted memory on empty maps and sets #29742
Avoid wasted memory on empty maps and sets #29742
Conversation
8aa642e
to
f41d348
Compare
spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java
Outdated
Show resolved
Hide resolved
Any specific reason to use |
Would you be so kind to explain the difference? Both methods return a static instance of an empty map/set. I used Set/Map.of() for the sake of brevity, and do not see any performance benefit in using either Set/Map.of() or Collections.emptySet/Map() |
The difference between |
wow, thanks. |
spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java
Outdated
Show resolved
Hide resolved
Aside from the |
Can you elaborate? AFAIK Map.of returns a static instance that is optimised for empty case: https://github.com/openjdk/jdk/blob/679e485838881c1364845072af305fb60d95e60a/src/java.base/share/classes/java/util/Map.java#L1333 |
Replaced Set.of() and Map.of with Collections.emptyXXX in order to avoid possible compatibility breaks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
Frankly speaking, I incline that adding early return in checkConfigMembers
would make the code slightly easier to follow.
spring-beans/src/main/java/org/springframework/beans/factory/annotation/InjectionMetadata.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for me (I’m just not quite sure about the impact of the code changes from high level, so let someone else to review and approve)
Should this change be ported to 5.3? |
We do not intend to backport minor enhancements such as this to |
Empty LinkedHashMaps waste quite a lot of memory (250kb) for a medium size project
The PR introduces a simple fix
A screen from yourkit profiler: