Skip to content
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 resizing of fixed-size HashSet/LinkedHashSet variants #32291

Closed
wants to merge 1 commit into from

Conversation

kilink
Copy link
Contributor

@kilink kilink commented Feb 18, 2024

Add helpers to CollectionUtils for building HashSets and LinkedHashSets that can hold an expected number of elements without needing to resize / rehash.

I also went ahead and updated places throughout the code base that appeared to be potentially sizing HashSet / LinkedHashSet incorrectly. This is by no means exhaustive.

See ff11467, where this was added for HashMap / LinkedHashMap.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 18, 2024
public static <E> LinkedHashSet<E> newLinkedHashSet(int expectedSize) {
return new LinkedHashSet<>(computeMapInitialCapacity(expectedSize), DEFAULT_LOAD_FACTOR);
}

private static int computeMapInitialCapacity(int expectedSize) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static int computeMapInitialCapacity(int expectedSize) {
private static int computeInitialCapacity(int expectedSize) {

Probably better to rename this method since it's now used for maps and sets.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that, but noticed that Guava and the JDK both refer to map capacity helpers in their versions of this helper. It sort of makes sense I guess since it ends up being backed by a Map. Don't really have a strong opinion so I'll make your suggested change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sort of makes sense I guess since it ends up being backed by a Map.

True. I just thought the new name would require less cognitive overhead for anyone reading the code. Though, to be honest, it's an internal helper method. So it doesn't really matter.

In any case, thanks for making the change.

On a side note, I just learned that there's a new HashSet.newHashSet() factory method in Java 19. So thanks for that. 👍

Though we still have a Java 17 baseline.

@sbrannen sbrannen self-assigned this Feb 19, 2024
@sbrannen sbrannen 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 Feb 19, 2024
@sbrannen sbrannen added this to the 6.2.x milestone Feb 19, 2024
Add helpers to CollectionUtils for building HashSets and LinkedHashSets
that can hold an expected number of elements without needing to resize / rehash.
@kilink
Copy link
Contributor Author

kilink commented Feb 19, 2024

I also wanted to note that part of the reason for this PR is that I'd like to be able to rely on this helper in Spring projects that can't use the JDK19 helpers, and where we don't necessarily want to rely on Guava.

@jhoeller jhoeller modified the milestones: 6.2.x, 6.2.0-M1 Feb 19, 2024
* @param expectedSize the expected number of elements (with a corresponding
* capacity to be derived so that no resize/rehash operations are needed)
* @see #newLinkedHashSet(int)
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, please remember to add @since tags to newly introduced non-private` methods and to update the copyright headers of all affected files.

Though no need to do that for this PR: I'll take care of that when merging.

@sbrannen sbrannen closed this in e1a32d4 Feb 21, 2024
sbrannen added a commit that referenced this pull request Feb 21, 2024
sbrannen added a commit that referenced this pull request Feb 21, 2024
@sbrannen
Copy link
Member

This has been merged into main in e1a32d4, revised in b9c304b, and extended in 644887e.

Thanks

@sbrannen sbrannen changed the title Avoid resizing of fixed-size HashSet/LinkedHashSet variants Avoid resizing of fixed-size HashSet/LinkedHashSet variants Mar 10, 2024
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.

4 participants