RFR JDK-6321472: Add CRC-32C API
Alan.Bateman at oracle.com
Fri Oct 17 11:05:39 UTC 2014
On 17/10/2014 02:42, Staffan Friberg wrote:
> This RFE adds a CRC-32C class. It implements Checksum so it will have
> the same API CRC-32, but use a different polynomial when calculating
> the CRC checksum.
> CRC-32C implementation uses slicing-by-8 to achieve high performance
> when calculating the CRC value.
> A part from adding the new class, java.util.zip.CRC32C, I have also
> added two default methods to Checksum. These are methods that were
> added to Adler32 and CRC32 in JDK 8 but before default methods were
> added, which was why they were only added to the implementors and not
> the interface.
> Bug: https://bugs.openjdk.java.net/browse/JDK-6321472
> Webrev: http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.00
I looked over the javadoc, I haven't found time to study the
implementation in CRC32C closely yet. Hopefully Sherman will be able to
review as he I think he has prototyped several CRC32C implementations.
On Checksum#update(ByteBuffer) then a suggestion for:
"The checksum is updated using buffer.remaining, starting a
is to replace it with:
"The checksum is updated with the remaining bytes in the buffer,
starting at the buffer's position."
In the @implNote then I wonder if it would be better to just leave out
the note about when the invariants are broken, we don't do that in other
places where breakage this is detected. Also I wonder if the note about
"For best performance, ..." should be an @apiNote.
Should CRC32C be final unless we have a good reason for it not to be final?
In CRC32C then I assume you don't need the <p>/<p> for the first
statement. The @see Checksum might not be too interesting here given
that it will be linked to anyway by way of implement Checksum.
I think it's more usual to list the @param tags before the @throws.
For the bounds check then you might want to look at the wording in other
areas (in java.lang or java.io for example) to get this better wording
(as off+len can overflow).
A minor comment on CRC32C is that you might want to keep the line
lengths consistent with the rest of the code. I only mention is for
future side-by-side reviews to make it a bit easier and avoid too much
More information about the core-libs-dev