From 9415bb7bdb42d8012ca457a90070b616e6bbec19 Mon Sep 17 00:00:00 2001 From: benbenw Date: Mon, 14 Sep 2020 17:26:03 +0200 Subject: [PATCH] fix: When passing a sub-array (offset, length) to the Storage#create method the array is needlessly cloned (#506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit leading to unnecessary memory allocations. No new tests as it was already covered. Signed-off-by: benoit Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [X] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/java-storage/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [X] Ensure the tests and linter pass - [X] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) Fixes #505 ☕️ --- .../com/google/cloud/storage/StorageImpl.java | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java index b1f2bfe3c3..c3778a8175 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java @@ -154,7 +154,7 @@ public Blob create(BlobInfo blobInfo, BlobTargetOption... options) { .setMd5(EMPTY_BYTE_ARRAY_MD5) .setCrc32c(EMPTY_BYTE_ARRAY_CRC32C) .build(); - return internalCreate(updatedInfo, EMPTY_BYTE_ARRAY, options); + return internalCreate(updatedInfo, EMPTY_BYTE_ARRAY, 0, 0, options); } @Override @@ -168,23 +168,26 @@ public Blob create(BlobInfo blobInfo, byte[] content, BlobTargetOption... option BaseEncoding.base64() .encode(Ints.toByteArray(Hashing.crc32c().hashBytes(content).asInt()))) .build(); - return internalCreate(updatedInfo, content, options); + return internalCreate(updatedInfo, content, 0, content.length, options); } @Override public Blob create( BlobInfo blobInfo, byte[] content, int offset, int length, BlobTargetOption... options) { content = firstNonNull(content, EMPTY_BYTE_ARRAY); - byte[] subContent = Arrays.copyOfRange(content, offset, offset + length); BlobInfo updatedInfo = blobInfo .toBuilder() - .setMd5(BaseEncoding.base64().encode(Hashing.md5().hashBytes(subContent).asBytes())) + .setMd5( + BaseEncoding.base64() + .encode(Hashing.md5().hashBytes(content, offset, length).asBytes())) .setCrc32c( BaseEncoding.base64() - .encode(Ints.toByteArray(Hashing.crc32c().hashBytes(subContent).asInt()))) + .encode( + Ints.toByteArray( + Hashing.crc32c().hashBytes(content, offset, length).asInt()))) .build(); - return internalCreate(updatedInfo, subContent, options); + return internalCreate(updatedInfo, content, offset, length, options); } @Override @@ -199,7 +202,12 @@ public Blob create(BlobInfo blobInfo, InputStream content, BlobWriteOption... op return Blob.fromPb(this, storageRpc.create(blobPb, inputStreamParam, optionsMap)); } - private Blob internalCreate(BlobInfo info, final byte[] content, BlobTargetOption... options) { + private Blob internalCreate( + BlobInfo info, + final byte[] content, + final int offset, + final int length, + BlobTargetOption... options) { Preconditions.checkNotNull(content); final StorageObject blobPb = info.toPb(); final Map optionsMap = optionMap(info, options); @@ -210,7 +218,8 @@ private Blob internalCreate(BlobInfo info, final byte[] content, BlobTargetOptio new Callable() { @Override public StorageObject call() { - return storageRpc.create(blobPb, new ByteArrayInputStream(content), optionsMap); + return storageRpc.create( + blobPb, new ByteArrayInputStream(content, offset, length), optionsMap); } }, getOptions().getRetrySettings(),