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

Use Arrays.compareUnsigned instead of iterating compare. #13252

Merged
merged 24 commits into from
Apr 19, 2024

Conversation

vsop-479
Copy link
Contributor

Similar to SegmentTermsEnumFrame, we can use Arrays.compareUnsigned instead of iterating compare, in SegmentTermsEnum.seekCeil and SegmentTermsEnum.seekExact.

@vsop-479
Copy link
Contributor Author

@mikemccand

Please take a look when you get a chance.

@rmuir
Copy link
Member

rmuir commented Mar 30, 2024

This looks like a good improvement, these Arrays methods become much faster than the home-written loops for longer arrays.

Copy link
Contributor

@uschindler uschindler left a comment

Choose a reason for hiding this comment

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

Great. Better replace all those handwritten loops with Arrays methods. We did this for the first time to improve Lucene 8.x with Java9+ (using MR-JAR).

Copy link
Member

@mikemccand mikemccand left a comment

Choose a reason for hiding this comment

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

Looks great, thank you @vsop-479! I'll merge soon.

@mikemccand
Copy link
Member

Woops, there are now conflicts here (from the binary search PR) -- maybe you could resolve them @vsop-479, and add a CHANGES.txt entry too? Thanks!

@vsop-479
Copy link
Contributor Author

vsop-479 commented Apr 1, 2024

I will resolve the conflicts, and try to find other handwritten loops. Thanks @mikemccand @uschindler .

@vsop-479
Copy link
Contributor Author

vsop-479 commented Apr 8, 2024

@mikemccand
I resolved the conflicts, and added a CHANGES.txt entry.
Please take a look when you get a chance.

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.

4 participants