smengcl commented on code in PR #10350:
URL: https://github.com/apache/ozone/pull/10350#discussion_r3771660649
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/Checksum.java:
##########
@@ -218,65 +268,71 @@ public ChecksumData computeChecksum(ChunkBuffer data)
return computeChecksum(data, false);
}
+ /**
+ * This method does not advance the positions of {@code data}'s underlying
+ * buffers. Both the no-cache and cache paths slice via
+ * {@link ByteBuffer#duplicate()}.
+ */
public ChecksumData computeChecksum(ChunkBuffer data, boolean useCache)
throws OzoneChecksumException {
if (checksumType == ChecksumType.NONE) {
- // Since type is set to NONE, we do not need to compute the checksums
return new ChecksumData(checksumType, bytesPerChecksum);
}
- final Function<ByteBuffer, ByteString> function;
+ final StreamingChecksum algo;
try {
- function = Algorithm.valueOf(checksumType).newChecksumFunction();
+ algo = Algorithm.valueOf(checksumType).newStreamingChecksum();
} catch (Exception e) {
- throw new OzoneChecksumException("Failed to get the checksum function
for " + checksumType, e);
+ throw new OzoneChecksumException(
+ "Failed to create streaming checksum for " + checksumType, e);
}
- final List<ByteString> checksumList;
- if (checksumCache == null || !useCache) {
- // When checksumCache is not enabled:
- // Checksum is computed for each bytesPerChecksum number of bytes of data
- // starting at offset 0. The last checksum might be computed for the
- // remaining data with length less than bytesPerChecksum.
- checksumList = new ArrayList<>();
- for (ByteBuffer b : data.iterate(bytesPerChecksum)) {
- checksumList.add(computeChecksum(b, function, bytesPerChecksum)); //
merge this?
- }
- } else {
- // When checksumCache is enabled:
- // We only need to update the last checksum in the cache, then pass it
along.
- checksumList = checksumCache.computeChecksum(data, function);
- }
+ final List<ByteString> checksumList = (checksumCache == null || !useCache)
+ ? computeChecksumDirect(data, algo)
+ : checksumCache.computeChecksum(data, algo, bytesPerChecksum);
return new ChecksumData(checksumType, bytesPerChecksum, checksumList);
}
/**
- * Compute checksum using the algorithm for the data upto the max length.
- * @param data input data
- * @param function the checksum function
- * @param maxLength the max length of data
- * @return computed checksum ByteString
+ * Walk {@code data}'s underlying ByteBuffer list, slicing each window of
+ * {@link #bytesPerChecksum} bytes via {@link ByteBuffer#duplicate()} and
+ * feeding slices to {@code algo}. No linearization byte[] is allocated
+ * when a window straddles multiple buffers.
*/
- protected static ByteString computeChecksum(ByteBuffer data,
- Function<ByteBuffer, ByteString> function, int maxLength) {
- final int limit = data.limit();
- try {
- final int maxIndex = data.position() + maxLength;
- if (limit > maxIndex) {
- data.limit(maxIndex);
+ private List<ByteString> computeChecksumDirect(ChunkBuffer data,
+ StreamingChecksum algo) {
+ final int dataLength = data.remaining();
+ final int checksumCount = dataLength == 0 ? 0 : 1 + (dataLength - 1) /
bytesPerChecksum;
Review Comment:
The suggested form is simpler but the addition could overflow because both
values are int.
e.g. with `dataLength = Integer.MAX_VALUE` and `bytesPerChecksum = 16384`,
it produces a negative count. The current expression calculates the same
ceiling division without overflow, so I prefer to keep it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
