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

SOLR-16989: Optimize and consolidate reuse of DocValues iterators for value retrieval #1938

Merged

Conversation

magibney
Copy link
Contributor

See: SOLR-16989

@magibney
Copy link
Contributor Author

Proposed CHANGES.txt entry under Optimizations for 9.4.0 release:

* SOLR-16989: Optimize and consolidate reuse of DocValues iterators for value retrieval (Michael Gibney)

@magibney
Copy link
Contributor Author

I have compared export performance on a large shard (using various sorts) against a dense field (required field id, which would not benefit at all from this optimization and could only stand to lose), and performance/latency are basically equivalent.

This stands to reason, because the overhead is basically a few int comparisons and primitive array assignments. Anyway the upshot is that for export value retrieval there should be basically no cost to introducing this in the case of dense fields, and improvements (potentially significant) to introducing this for sparse fields.

In the SolrDocumentFetcher case there should be significant improvements for all cases.

I plan to commit this early next week, pending feedback.

@magibney magibney merged commit dafbe80 into apache:main Sep 26, 2023
1 of 2 checks passed
magibney added a commit that referenced this pull request Sep 26, 2023
Copy link
Contributor

@dsmiley dsmiley left a comment

Choose a reason for hiding this comment

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

I don't see a code reviewer. FWIW I would have loved to reviewed this at the time; I have history in SolrDocumentFetcher.

@magibney
Copy link
Contributor Author

I would have loved the review, sorry this didn't get on your radar at the time. Please lmk if you have any concerns about what was committed, or what could/should have been committed alongside; I certainly don't mind revisiting.

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