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

Integrate merge-time index reordering with the intra-merge executor. #13289

Merged
merged 4 commits into from
Sep 12, 2024

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Apr 10, 2024

Index reordering can benefit greatly from parallelism, so it should try to use the intra-merge executor when possible. Because the API only exposes an Executor and not a ForkJoinPool, parallelism is a bit less good due to some threads potentially waiting for tasks to finish when they could be doing work stealing instead. We should look into whether we can improve this in the future.

Index reordering can benefit greatly from parallelism, so it should try to use
the intra-merge executor when possible. Because the API only exposes an
`Executor` and not a `ForkJoinPool`, parallelism is a bit less good due to some
threads potentially waiting for tasks to finish when they could be doing work
stealing instead. We should look into whether we can improve this in the
future.
@jpountz jpountz added this to the 10.0.0 milestone Apr 10, 2024
Copy link

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

@github-actions github-actions bot added the Stale label Apr 25, 2024
@jpountz
Copy link
Contributor Author

jpountz commented Sep 11, 2024

I improved the PR by taking advantage of TaskExecutor's work stealing logic. I plan on merging soon if there are no objections.

@benwtrent
Copy link
Member

@jpountz I think this is a good idea. One concern is that the CMS may not actually use parallelism on tiny merges. Let me separate out the change in this PR to improve our testing around this: https://github.com/apache/lucene/pull/13475/files

@github-actions github-actions bot removed the Stale label Sep 12, 2024
@jpountz jpountz merged commit ff8b81a into apache:main Sep 12, 2024
3 checks passed
@jpountz jpountz modified the milestones: 10.0.0, 9.12.0 Sep 12, 2024
jpountz added a commit that referenced this pull request Sep 12, 2024
…13289)

Index reordering can benefit greatly from parallelism, so it should try to use
the intra-merge executor when possible.
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.

2 participants