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

Add reopen method in PerThreadPKLookup #13596

Merged
merged 13 commits into from
Oct 10, 2024
Merged

Conversation

vsop-479
Copy link
Contributor

@vsop-479 vsop-479 commented Jul 21, 2024

Description

Reuse loaded segments' termsEnum and postingsEnum.

@vsop-479 vsop-479 marked this pull request as draft July 21, 2024 08:21
@vsop-479 vsop-479 changed the title [WIP] Add reopen method in PerThreadPKLookup Add reopen method in PerThreadPKLookup Jul 24, 2024
@vsop-479 vsop-479 marked this pull request as ready for review July 24, 2024 06:50
@vsop-479
Copy link
Contributor Author

@jpountz

Please take a look when you get a chance.

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 Aug 17, 2024
@vsop-479
Copy link
Contributor Author

@jpountz
Please take a look when you get a chance.

@github-actions github-actions bot removed the Stale label Aug 20, 2024
@vsop-479
Copy link
Contributor Author

I can't figure out what can we reuse from prior PostingsEnum.

If we want reuse prior PostingsEnum's docBuffer( if it still stay in first block), we should not reset docBufferUpto to BLOCK_SIZE (maybe set to 0?) when prior docStartFP equals current's, which will result in refillDocs.

For the situation of PK, every PK has different PostingsEnum, prior PostingsEnum is mostly useless if reuse means reuse same PostingsEnum loaded docBuffer.

@jpountz Please correct me if I am wrong.

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.

I wanted to look at your PR locally to understand what it did. It looked good to me but I saw a few simplifications I wanted to make, such as not caching null TermsEnum, since returning null Terms is not expensive on a reader. I felt free to push to your branch, I hope you don't mind. The change looks good to me.

@vsop-479
Copy link
Contributor Author

This change looks clearer, Thanks @jpountz .

Copy link

github-actions bot commented Oct 4, 2024

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!

@jpountz jpountz added this to the 10.1.0 milestone Oct 10, 2024
@jpountz jpountz merged commit ba72d22 into apache:main Oct 10, 2024
3 checks passed
jpountz added a commit that referenced this pull request Oct 10, 2024
Co-authored-by: Adrien Grand <jpountz@gmail.com>
@vsop-479 vsop-479 deleted the add_reopen_PKLookup branch October 12, 2024 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants