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

Speedup SearchResponse serialization #123211

Merged

Conversation

original-brownbear
Copy link
Member

No need to have two nested concat iterators here (each of which adding a layer of megamorphic calls + pointer indirection on every next and hasNext()!!!). There's obviously still lots and lots of room for optimization on this one, but just flattening out two obvious steps here enormously reduces the number of method calls required when serializing a search response. Given that method calls can consume up to half the serialization cost this change might massively speed up some use cases.

No need to have a nested concat here. There's obviously lots and lots of
room for optimization on this one, but just flattening out one obvious
step here outright halves the number of method calls required when
serializing a search response. Given that method calls can consume up to
half the serialization cost this change might massively speed up some
usecases.
@original-brownbear original-brownbear added >non-issue :Search Foundations/Search Catch all for Search Foundations labels Feb 22, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

@elasticsearchmachine elasticsearchmachine added Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v9.1.0 labels Feb 22, 2025
return Iterators.concat(
wrapInObject ? ChunkedToXContentHelper.startObject() : Collections.emptyIterator(),
Copy link
Member Author

Choose a reason for hiding this comment

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

Very lazy solution admittedly here and for the end-object but it doesn't matter as the cost comes from the hits iterator anyway.

@original-brownbear original-brownbear added auto-backport Automatically create backport pull requests when merged v8.19.0 labels Feb 24, 2025
@original-brownbear
Copy link
Member Author

Thanks Luca!

@original-brownbear original-brownbear merged commit 236b955 into elastic:main Feb 24, 2025
17 checks passed
@original-brownbear original-brownbear deleted the tiny-fix-search-response branch February 24, 2025 11:00
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 123211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged backport pending >non-issue :Search Foundations/Search Catch all for Search Foundations Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch v8.19.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants