Skip to content

Commit

Permalink
fix: When passing a sub-array (offset, length) to the Storage#create …
Browse files Browse the repository at this point in the history
…method the array is needlessly cloned (#506)

 leading to unnecessary memory allocations.

No new tests as it was already covered.

Signed-off-by: benoit <b.wiart@ubik-ingenierie.com>

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 ☕️
  • Loading branch information
benbenw authored Sep 14, 2020
1 parent 0e58c1c commit 9415bb7
Showing 1 changed file with 17 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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<StorageRpc.Option, ?> optionsMap = optionMap(info, options);
Expand All @@ -210,7 +218,8 @@ private Blob internalCreate(BlobInfo info, final byte[] content, BlobTargetOptio
new Callable<StorageObject>() {
@Override
public StorageObject call() {
return storageRpc.create(blobPb, new ByteArrayInputStream(content), optionsMap);
return storageRpc.create(
blobPb, new ByteArrayInputStream(content, offset, length), optionsMap);
}
},
getOptions().getRetrySettings(),
Expand Down

0 comments on commit 9415bb7

Please sign in to comment.