Skip to content

Use HashMap#newHashMap, HashSet#newHashSet instead of CollectionUtil#newHashMap, CollectionUtil#newHashSet.#15517

Merged
vsop-479 merged 2 commits intoapache:mainfrom
vsop-479:use_HashMap#newHashMap_HashSet#newHashSet
Dec 25, 2025
Merged

Use HashMap#newHashMap, HashSet#newHashSet instead of CollectionUtil#newHashMap, CollectionUtil#newHashSet.#15517
vsop-479 merged 2 commits intoapache:mainfrom
vsop-479:use_HashMap#newHashMap_HashSet#newHashSet

Conversation

@vsop-479
Copy link
Copy Markdown
Contributor

Description

Discussed in #14770.

@vsop-479
Copy link
Copy Markdown
Contributor Author

Hello, @uschindler
There is a little difference between two versions on the calculation fo capacity: the old one divide by 0.75f, and add one. Now , we just divide 0.75f.

@vsop-479 vsop-479 requested a review from uschindler December 23, 2025 09:05
@uschindler
Copy link
Copy Markdown
Contributor

Hi,
The plan was to remove both methods in main branch and replace the whole code by the default variant I n JDK.

The plus1 issue comes from a safety added, I think because @thecoop wanted to make sure that after rounding the size is correct. According to JDK docs the newHashXxx method in jdk ensures that the requested elements fit into the map without resizing. So they .ay gave changed other stuff to make the plus1 not needed.

Uwe

@thecoop
Copy link
Copy Markdown
Contributor

thecoop commented Dec 23, 2025

Yup, these CollectionUtil methods can just be removed and the JCL methods used directly. There'll need to be a changelog entry documenting the API removal. CollectionUtil is marked as internal, so we can just remove these for Lucene 11

@vsop-479
Copy link
Copy Markdown
Contributor Author

Thanks @uschindler, @thecoop.

I considered use the JDK methods directly, but chosed this way.

Anyway, I will fix it.

@github-actions github-actions Bot added this to the 11.0.0 milestone Dec 24, 2025
@vsop-479 vsop-479 changed the title Use HashMap#newHashMap, HashSet#newHashSet in CollectionUtil. Use HashMap#newHashMap, HashSet#newHashSet instead of CollectionUtil#newHashMap, CollectionUtil#newHashSet. Dec 24, 2025
@vsop-479
Copy link
Copy Markdown
Contributor Author

Hi @uschindler , @thecoop
I removed CollectionUtil#newHashMap, CollectionUtil#newHashSet, and use Use HashMap#newHashMap, HashSet#newHashSet directly.

This pr also plus 1 for CustomAnalyzer#paramsToMap 's map size.

}
final Map<String, String> map = CollectionUtil.newHashMap(params.length / 2);
// Plus 1 since applyDefaultParams will add a default param later.
final Map<String, String> map = HashMap.newHashMap(params.length / 2 + 1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@uschindler
Copy link
Copy Markdown
Contributor

Merry Xmas! 🎄

@vsop-479
Copy link
Copy Markdown
Contributor Author

Merry Xmas! 🎄

I will backport this to 10.x

@uschindler
Copy link
Copy Markdown
Contributor

For 10.x maybe keep the CollectionUtil method as deprecated.

@vsop-479 vsop-479 merged commit 02c40ba into apache:main Dec 25, 2025
13 checks passed
@vsop-479 vsop-479 deleted the use_HashMap#newHashMap_HashSet#newHashSet branch December 25, 2025 05:38
vsop-479 added a commit that referenced this pull request Dec 25, 2025
…il#newHashMap and CollectionUtil#newHashSet. (#15517). But keep CollectionUtil's methods as deprecated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants