Hi David,
Thanks for reviewing.
CRC32C.java:
56 /**
57 * CRC-32C Polynom
58 */
“Polynomial”, perhaps?
Done
You did test this on both big- and little-endian boxes, right?
It looks right to me, but this stuff is brain-hurty on mirror-image dyslexia
and fenceposts.
Yes, as part of the push I added a tests for both CRC32 and CRC32C and
they pass on all platforms (and were really helpful when implementing).
It might not hurt to mention that byteTable is for byte-at-a-time calculations
and is always
the same, whereas the other 8 tables are for bulk operations and will have
contents
dependent on the endianness of the platform.
Did a slight reorder to separate the single table and the bulk tables
and added comments according to your suggestion.
It would be a good idea to mention that the high-order terms of polynomials
are stored in the low-order bits of the integers in the tables.
You could get the bit reversal out of the init code if you bit-reversed
CRC32C_POLY instead and wrote it
78 int r = index;
// High-order polynomial term stored in LSB of r.
79 for (int i = 0; i < Byte.SIZE; i++) {
80 if ((r & 1) != 0) {
81 r = (r >>> 1) ^ REVERSED_CRC32C_POLY;
82 } else {
83 r >>>= 1;
84 }
85 }
86 byteTables[0][index] = r;
at least, it seems that way to me.
Agree, and so does the tests... :)
Here is the last webrev with the above changes,
http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.09
I will ask Sherman to sponsor and push the change, since Joe mentioned
only a single Reviewer is currently OK in JDK 9.
Thanks,
Staffan
On 11/20/2014 08:41 PM, David Chase wrote:
On 2014-11-20, at 7:45 AM, Staffan Friberg <staffan.frib...@oracle.com> wrote:
Hi,
Anyone who can be the second Reviewer?
Thanks,
Staffan
I can review, but I am not a Reviewer.
CRC32C.java:
56 /**
57 * CRC-32C Polynom
58 */
“Polynomial”, perhaps?
You did test this on both big- and little-endian boxes, right?
It looks right to me, but this stuff is brain-hurty on mirror-image dyslexia
and fenceposts.
It might not hurt to mention that byteTable is for byte-at-a-time calculations
and is always
the same, whereas the other 8 tables are for bulk operations and will have
contents
dependent on the endianness of the platform.
It would be a good idea to mention that the high-order terms of polynomials
are stored in the low-order bits of the integers in the tables.
You could get the bit reversal out of the init code if you bit-reversed
CRC32C_POLY instead and wrote it
78 int r = index;
// High-order polynomial term stored in LSB of r.
79 for (int i = 0; i < Byte.SIZE; i++) {
80 if ((r & 1) != 0) {
81 r = (r >>> 1) ^ REVERSED_CRC32C_POLY;
82 } else {
83 r >>>= 1;
84 }
85 }
86 byteTables[0][index] = r;
at least, it seems that way to me.
David
