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

simple cleanups to vector code #12680

Merged
merged 2 commits into from
Oct 15, 2023
Merged

simple cleanups to vector code #12680

merged 2 commits into from
Oct 15, 2023

Conversation

rmuir
Copy link
Member

@rmuir rmuir commented Oct 14, 2023

Now that we have integrated benchmarks, it is easier to take care of this code.

This is pretty straightforward change:

  • split out vectorized loops to avoid huge methods (especially integer methods with multiple implementations)
  • a couple variable renames and "import static" so that formatter stops butchering the file and making it unreadable
  • correct comments where possible while i'm here, i know, still not perfect

I see the same performance (no logic was changed)

Note: not much can be done with cosine method, for obvious reasons that it is a slow complex nightmare, IMO we shouldn't support it anyway. But we can't let that get in the way of trying to improve the other ones.

@rmuir
Copy link
Member Author

rmuir commented Oct 14, 2023

cosine() ones cleaned up now too. I don't see perf issue with the array: guess this whole shebang relies on escape analysis anyway.

Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

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

Much cleaner ( I had a similar thought). LGTM

@rmuir rmuir merged commit db746bd into apache:main Oct 15, 2023
4 checks passed
@uschindler uschindler added this to the 9.9.0 milestone Oct 15, 2023
@uschindler uschindler self-assigned this Oct 15, 2023
@uschindler
Copy link
Contributor

I backported this one to 9.x. As I cleaned up commits, this one was committed in 9.x as part of #12681

clayburn added a commit to runningcode/lucene that referenced this pull request Oct 20, 2023
…ache.org

* upstream/main: (239 commits)
  Bound the RAM used by the NodeHash (sharing common suffixes) during FST compilation (apache#12633)
  Fix index out of bounds when writing FST to different metaOut (apache#12697) (apache#12698)
  Avoid object construction when linear searching arcs (apache#12692)
  chore: update the Javadoc example in Analyzer (apache#12693)
  coorect position on entry in CHANGES.txt
  Refactor ByteBlockPool so it is just a "shift/mask big array" (apache#12625)
  Extract the hnsw graph merging from being part of the vector writer (apache#12657)
  Specialize `BlockImpactsDocsEnum#nextDoc()`. (apache#12670)
  Speed up TestIndexOrDocValuesQuery. (apache#12672)
  Remove over-counting of deleted terms (apache#12586)
  Use MergeSorter in StableStringSorter (apache#12652)
  Use radix sort to speed up the sorting of terms in TermInSetQuery (apache#12587)
  Add timeouts to github jobs. Estimates taken from empirical run times (actions history), with a generous buffer added. (apache#12687)
  Optimize OnHeapHnswGraph's data structure (apache#12651)
  Add createClassLoader to replicator permissions (block specific to jacoco). (apache#12684)
  Move changes entry before backporting
  CHANGES
  Move testing properties to provider class (no classloading deadlock possible) and fallback to default provider in non-test mode
  simple cleanups to vector code (apache#12680)
  Better detect vector module in non-default setups (e.g., custom module layers) (apache#12677)
  ...
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