Use IntArrayList/IntHashSet to replace usages of List/Set of Integer - #14774
Conversation
github-project-automation
Bot
moved this to Open
in OpenSearch Lucene & Core Performance Tracking
|
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR. |
dweiss
left a comment
dweiss
left a comment
There was a problem hiding this comment.
LGTM. I think it's fine to change the signature of those low-level methods.
benwtrent
left a comment
benwtrent
left a comment
There was a problem hiding this comment.
Could you add a change log note? I realize that this is more of a refactoring/performance thing.
| * @return a set of nodes that best cover the graph | ||
| */ | ||
| public static Set<Integer> computeJoinSet(HnswGraph graph) throws IOException { | ||
| public static IntHashSet computeJoinSet(HnswGraph graph) throws IOException { |
There was a problem hiding this comment.
Honestly, this whole class should be marked as internal.
I think making this change is perfectly fine.
There was a problem hiding this comment.
Thanks for the guidance. I’ve added @lucene.internal for this class.
|
@dweiss @benwtrent Thanks for the quick review, I have added the CHANGES entry under 10.3.0 |
# Conflicts: # lucene/CHANGES.txt
github-project-automation
Bot
moved this from Open
to Merged
in OpenSearch Lucene & Core Performance Tracking
…14774) * Replace List<Integer> by IntArrayList * Replace Set<Integer> by IntHashSet * mark UpdateGraphsUtils as lucene internal
3 participants
No functional changes — only optimization to reduce auto-boxing. However, this involves a public API change in
UpdateGraphsUtils#computeJoinSet. not sure whether we should touch this public API?