Skip to content

Add safer conversion from RecyclerBytesStreamOutput to ReleasableBytesReference #127404

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

Merged

Conversation

original-brownbear
Copy link
Contributor

We have a couple of places in the codebase where we do the transition from the stream to the reference.
We can save some code and make this a little less error-prone by having a conversion method with move-style semantics and enabling the use of try-with-resources. Also, this enables a couple of optimizations down the line and unlinking the list of pages and moving it to the reference instead of nulling it out is a bit nicer to the CPU caches also.

…sReference

We have a couple of places in the codebase where we do the transition from the stream
to the reference.
We can save some code and make this a little less error-prone by having a conversion
method with move-style semantics and enabling the use of try-with-resources.
Also, this enables a couple of optimizations down the line and unlinking the list of pages
and moving it to the reference instead of nulling it out is a bit nicer to the CPU caches
also.
@original-brownbear original-brownbear added >non-issue :Distributed Coordination/Network Http and internode communication implementations labels Apr 25, 2025
@original-brownbear original-brownbear requested a review from a team as a code owner April 25, 2025 16:04
@elasticsearchmachine elasticsearchmachine added Team:Distributed Coordination Meta label for Distributed Coordination team v9.1.0 labels Apr 25, 2025
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@rjernst rjernst 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 Ryan!

@original-brownbear original-brownbear merged commit 8960efb into elastic:main May 6, 2025
17 checks passed
@original-brownbear original-brownbear deleted the harder-recycler-stream branch May 6, 2025 19:00
parkertimmins pushed a commit to parkertimmins/elasticsearch that referenced this pull request May 7, 2025
…sReference (elastic#127404)

We have a couple of places in the codebase where we do the transition from the stream
to the reference.
We can save some code and make this a little less error-prone by having a conversion
method with move-style semantics and enabling the use of try-with-resources.
Also, this enables a couple of optimizations down the line and unlinking the list of pages
and moving it to the reference instead of nulling it out is a bit nicer to the CPU caches
also.
ywangd pushed a commit to ywangd/elasticsearch that referenced this pull request May 9, 2025
…sReference (elastic#127404)

We have a couple of places in the codebase where we do the transition from the stream
to the reference.
We can save some code and make this a little less error-prone by having a conversion
method with move-style semantics and enabling the use of try-with-resources.
Also, this enables a couple of optimizations down the line and unlinking the list of pages
and moving it to the reference instead of nulling it out is a bit nicer to the CPU caches
also.
afoucret pushed a commit to afoucret/elasticsearch that referenced this pull request May 9, 2025
…sReference (elastic#127404)

We have a couple of places in the codebase where we do the transition from the stream
to the reference.
We can save some code and make this a little less error-prone by having a conversion
method with move-style semantics and enabling the use of try-with-resources.
Also, this enables a couple of optimizations down the line and unlinking the list of pages
and moving it to the reference instead of nulling it out is a bit nicer to the CPU caches
also.
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request May 12, 2025
…sReference (elastic#127404)

We have a couple of places in the codebase where we do the transition from the stream
to the reference.
We can save some code and make this a little less error-prone by having a conversion
method with move-style semantics and enabling the use of try-with-resources.
Also, this enables a couple of optimizations down the line and unlinking the list of pages
and moving it to the reference instead of nulling it out is a bit nicer to the CPU caches
also.
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