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 a MergeSorter taking advantage of extra storage for StableMSBRadixSorter #12623

Merged
merged 10 commits into from
Oct 5, 2023

Conversation

gf2121
Copy link
Contributor

@gf2121 gf2121 commented Oct 4, 2023

Description

As StableMSBRadixSorter always requires a O(n) extra memory. We can use a MergeSorter taking advantage of the extra memory instead of InPlaceMergeSorter.

Benchmark

This reduces around 12% took of sorting ~10,000,000 timestamp values (LongPoint) during indexing.

  baseline candidate took diff
sort took 5333 4667 -12.49%

@gf2121 gf2121 marked this pull request as draft October 5, 2023 06:23
@gf2121 gf2121 marked this pull request as ready for review October 5, 2023 06:47
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

Nice!

}

private void merge(int from, int to, int mid) {
assert to > mid && mid > from;
Copy link
Contributor

Choose a reason for hiding this comment

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

In merge sort, it is common to check if the value at mid-1 is less than or equal to the value at mid, to save work in case the data is already (partially) sorted, maybe we could do that here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great advice! Thank you!

@gf2121 gf2121 merged commit 28f0885 into apache:main Oct 5, 2023
4 checks passed
asfgit pushed a commit that referenced this pull request Oct 5, 2023
s1monw pushed a commit to s1monw/lucene that referenced this pull request Oct 10, 2023
@gf2121 gf2121 added this to the 9.9.0 milestone Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants