Change B: Avoid copying byte array for ResponseBytes
#12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Once the `ByteArrayAsyncResponseTransformer` has gathered all the response bytes, we still need to wrap those bytes and the response object in a `ResponseBytes` instance - but if we use `ResponseBytes.fromByteArray()`, a whole new byte array will be allocated, which is bad for two reasons: * While copying, the JVM heap must briefly hold both the old & new byte arrays - roughly speaking, doubling the memory requirements. * Copying the bytes from one array to another takes a little bit of CPU time (obviously this varies: `System.arraycopy()` for 40MB of bytes takes ~2ms on my M1 machine). A faster, more memory efficient alternative to `ResponseBytes.fromByteArray()` is `ResponseBytes.fromByteArrayUnsafe()`, added to the AWS SDK in August 2020 with aws/aws-sdk-java-v2#1977 in response to aws/aws-sdk-java-v2#1959. The 'Unsafe' in the name is a warning to users of this method that the underlying byte array is _not_ copied, and so could be susceptible to badly-behaving code manipulating the contents of the byte array after the `ResponseBytes` is handed to the calling code. If only the trusted SDK code has access to the original byte array before `ResponseBytes` is handed over to the caller, and once the `ResponseBytes` instance is handed over to the caller, the SDK code has no further use for the original byte array, then it is safe to use `ResponseBytes.fromByteArrayUnsafe()` in the trusted SDK code, and return the resulting `ResponseBytes` to the user, saving a double-allocation of memory, and the CPU time for the copying of bytes. See also: * aws/aws-sdk-java-v2#1959 * aws/aws-sdk-java-v2#1977
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Once the
ByteArrayAsyncResponseTransformerhas gathered all the response bytes, we still need to wrap those bytes and the response object in aResponseBytesinstance - but if we useResponseBytes.fromByteArray(), a whole new byte array will be allocated, which is bad for two reasons:System.arraycopy()for 40MB of bytes takes ~2ms on my M1 machine).A faster, more memory efficient alternative to
ResponseBytes.fromByteArray()isResponseBytes.fromByteArrayUnsafe(), added to the AWS SDK in August 2020 with aws/aws-sdk-java-v2#1977 in response to aws/aws-sdk-java-v2#1959. The 'Unsafe' in the name is a warning to users of this method that the underlying byte array is not copied, and so could be susceptible to badly-behaving code manipulating the contents of the byte array after theResponseBytesis handed to the calling code.If only the trusted SDK code has access to the original byte array before
ResponseBytesis handed over to the caller, and once theResponseBytesinstance is handed over to the caller, the SDK code has no further use for the original byte array, then it is safe to useResponseBytes.fromByteArrayUnsafe()in the trusted SDK code, and return the resultingResponseBytesto the user, saving a double-allocation of memory, and the CPU time for the copying of bytes.See also:
SdkBytesandResponseByteswithout copying the data. aws/aws-sdk-java-v2#1977Required RAM: 357 MB