Skip to content

Requests should take ByteBuf as input data in efficient manner #1951

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.

Sign up for GitHub

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

Jump to bottom
Closed
dgolombek opened this issue Apr 3, 2024 · 1 comment · Fixed by #1952
Closed

Requests should take ByteBuf as input data in efficient manner #1951

dgolombek opened this issue Apr 3, 2024 · 1 comment · Fixed by #1952

Comments

@dgolombek
Copy link
Contributor

Currently if the the builder of an AsyncHttpClient.Request object has a Netty ByteBuf, they must first convert it to a ByteBuffer/byte[]/ByteArrayInputStream/etc to pass it in, which generally means at least one allocation and two memory copies (into the temp buffer and then back out). This is very inefficient in both memory and CPU. I propose a new NettyByteBufBody that extends NettyDirectBody and will submit a PR to that effect. Another possible approach would be to add a Request.getNettyBody() method that allowed callers to plug in anything that implemented the NettyBody interface, but that feels like a slightly more dangerous change.
dgolombek pushed a commit to dgolombek/async-http-client that referenced this issue Apr 3, 2024
Extend Request to support ByteBuf inputs
e611951 
This extends RequestBuilderBase+Request+NettyRequestFactory to allow
providing a Netty ByteBuf as input in an efficient manner, avoiding
having to convert that ByteBuf to a ByteBuffer and then back.

Fixes AsyncHttpClient#1951
This was referenced Apr 3, 2024
Merged
Closed
@dgolombek
Copy link
Contributor Author

@hyperxpro anything I can do to help get this reviewed and merged? I have some work time to help with this project, starting with a few things we need, but then can also help with backlog of bugs or whatever else makes sense. Thanks.

hyperxpro added a commit that referenced this issue Apr 15, 2024
@dgolombek @hyperxpro
Extend Request to support ByteBuf inputs (#1952)
b251d43 
* Extend Request to support ByteBuf inputs

This extends RequestBuilderBase+Request+NettyRequestFactory to allow
providing a Netty ByteBuf as input in an efficient manner, avoiding
having to convert that ByteBuf to a ByteBuffer and then back.

Fixes #1951

* Fix up test for repeatability

---------

Co-authored-by: David Golombek <[email protected]>
Co-authored-by: Aayush Atharva <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Extend Request to support ByteBuf inputs dgolombek/async-http-client
1 participant
@dgolombek