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.

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

Redundant byte array copying in AsyncRequestBody #1735

Closed
RoKish opened this issue Mar 25, 2020 · 5 comments
Closed

Redundant byte array copying in AsyncRequestBody #1735

RoKish opened this issue Mar 25, 2020 · 5 comments
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue

Comments

@RoKish
Copy link

RoKish commented Mar 25, 2020

Describe the bug

It doesn't make sense to copy a byte array in AsyncRequestBody#fromBytes and AsyncRequestBody#fromByteBuffer since the byte array is copied in the constructor of ByteArrayAsyncRequestBody afterwards.

Possible Solution

I think the best solution will be to copy the byte array in the constructor of ByteArrayAsyncRequestBody. Another constructor should be added to accept offset and length in order to support copying from ByteBuffer.

Your Environment

  • AWS Java SDK version used: 2.7.15
@RoKish RoKish added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 25, 2020
@debora-ito
Copy link
Member

@RoKish seems reasonable, but it's more an enhancement than a bug.

If you can send us a PR, we'll be happy to take a look.

@debora-ito debora-ito added feature-request A feature should be added or improved. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 25, 2020
@yasminetalby yasminetalby added the p3 This is a minor priority issue label Nov 14, 2022
This was referenced Apr 20, 2023
Merged
Open
@StephenFlavin StephenFlavin mentioned this issue Jun 30, 2023
Closed
12 tasks
@millems
Copy link
Contributor

millems commented Jul 11, 2023

Fixed by #3925

@millems millems closed this as completed Jul 11, 2023
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@millems
Copy link
Contributor

millems commented Jul 11, 2023

@all-contributors please add @StephenFlavin for code

@allcontributors allcontributors bot mentioned this issue Jul 11, 2023
Merged
@allcontributors
Copy link
Contributor

@millems

I've put up a pull request to add @StephenFlavin! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@debora-ito @RoKish @millems @yasminetalby