Skip to content

Conversation

original-brownbear
Copy link
Contributor

We can make slicing things like search hits where we cut many small buffers out of a large composite reference a lot more efficient in some cases by making the slice of a releasable reference itself releasable. This fixes the current situation where we could have composite buffer of many MB that is made up of e.g. 16k chunks but that we would retain in full if we were to slice even a single byte out of it in any position.

… of unused pages

We can make slicing things like search hits where we cut many small
buffers out of a large composite reference a lot more efficient in some
cases by making the slice of a releasable reference itself releasable.
This fixes the current situation where we could have composite buffer
of many MB that is made up of e.g. 16k chunks but that we would retain
in full if we were to slice even a single byte out of it in any position.
@original-brownbear original-brownbear added >non-issue :Distributed Coordination/Network Http and internode communication implementations labels Apr 4, 2025
@original-brownbear original-brownbear requested a review from a team as a code owner April 4, 2025 11:47
@elasticsearchmachine elasticsearchmachine added v9.1.0 Team:Distributed Coordination Meta label for Distributed Coordination team labels Apr 4, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

if (from == 0 && length() == length) {
return retain();
}
final BytesReference slice = delegate.slice(from, length);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Funny enough I found this looking into a different optimization that wouldn't build composite bytes references out of releasable ones by unwrapping. While that has its runtime advantages, this change seems much more helpful by saving memory and optimisations like #124947 (if taken a little further that is) could leverage this for even bigger gains.
As it stands, this change actually is quite helpful already when working with SearchHit instances sliced out of large buffers at the very least (there might be other cases where it's helpful as well). Also, it avoids a pointless round of ref-counting in the decoder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could also be taken further by making the logic aware of situations where the slice itself is made up of a subset of the releasable references that the parent instance consists of and only retaining those. Obviously then considerably increasing the complexity of the implementation for a likely limited gain in corner cases.

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM

@original-brownbear
Copy link
Contributor Author

Thanks David!

@original-brownbear original-brownbear merged commit c06cbb6 into elastic:main Apr 4, 2025
16 of 17 checks passed
@original-brownbear original-brownbear deleted the improve-releasable-ref-slicing branch April 4, 2025 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Network Http and internode communication implementations >non-issue Team:Distributed Coordination Meta label for Distributed Coordination team v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants