RFR JDK-6321472: Add CRC-32C API

Staffan Friberg staffan.friberg at oracle.com
Tue Oct 21 18:49:20 UTC 2014


Hi,

Got an offline comment that the package.html should be update as well to 
cover CRC-32C.

Otherwise there are no code changes in this new webrev.

http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.04

//Staffan

On 10/21/2014 10:28 AM, Staffan Friberg wrote:
> Hi Peter,
>
> Thanks for the comments..
>>
>>   217                 if (Unsafe.ADDRESS_SIZE == 4) {
>>   218                     // On 32 bit platforms read two ints 
>> instead of a single 64bit long
>>
>> When you're reading from byte[] using Unsafe (updateBytes), you have 
>> the option of reading 64bit values on 64bit platforms. When you're 
>> reading from DirectByteBuffer memory (updateDirectByteBuffer), you're 
>> only using 32bit reads.
> I will add a comment in the code for this decision. The reason is that 
> read a long results in slightly worse performance in this case, in 
> updateBytes it is faster. I was able to get it to run slightly faster 
> by working directly with the address instead of always adding address 
> + off, but this makes things worse in the 32bit case since all 
> calculation will now be using long variables. So using the getInt as 
> in the current code feels like the best solution as it strikes the 
> best balance between 32 and 64bit. Below is how updateByteBuffer 
> looked with the rewrite I mentioned.
>
>
>  ong address = ((DirectBuffer) buffer).address();
>  crc = updateDirectByteBuffer(crc, address + pos, address + limit);
>
>
>      private static int updateDirectByteBuffer(int crc, long adr, long 
> end) {
>
>         // Do only byte reads for arrays so short they can't be aligned
>         if (end - adr >= 8) {
>
>             // align on 8 bytes
>             int alignLength = (8 - (int) (adr & 0x7)) & 0x7;
>             for (long alignEnd = adr + alignLength; adr < alignEnd; 
> adr++) {
>                 crc = (crc >>> 8)
>                         ^ byteTable[(crc ^ UNSAFE.getByte(adr)) & 0xFF];
>             }
>
>             if (ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN) {
>                 crc = Integer.reverseBytes(crc);
>             }
>
>             // slicing-by-8
>             for (; adr < (end - Long.BYTES); adr += Long.BYTES) {
>                 int firstHalf;
>                 int secondHalf;
>                 if (Unsafe.ADDRESS_SIZE == 4) {
>                     // On 32 bit platforms read two ints instead of a 
> single 64bit long
>                     firstHalf = UNSAFE.getInt(adr);
>                     secondHalf = UNSAFE.getInt(adr + Integer.BYTES);
>                 } else {
>                     long value = UNSAFE.getLong(adr);
>                     if (ByteOrder.nativeOrder() == 
> ByteOrder.LITTLE_ENDIAN) {
>                         firstHalf = (int) value;
>                         secondHalf = (int) (value >>> 32);
>                     } else { // ByteOrder.BIG_ENDIAN
>                         firstHalf = (int) (value >>> 32);
>                         secondHalf = (int) value;
>                     }
>                 }
>                 crc ^= firstHalf;
>                 if (ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN) {
>                     crc = byteTable7[crc & 0xFF]
>                             ^ byteTable6[(crc >>> 8) & 0xFF]
>                             ^ byteTable5[(crc >>> 16) & 0xFF]
>                             ^ byteTable4[crc >>> 24]
>                             ^ byteTable3[secondHalf & 0xFF]
>                             ^ byteTable2[(secondHalf >>> 8) & 0xFF]
>                             ^ byteTable1[(secondHalf >>> 16) & 0xFF]
>                             ^ byteTable0[secondHalf >>> 24];
>                 } else { // ByteOrder.BIG_ENDIAN
>                     crc = byteTable0[secondHalf & 0xFF]
>                             ^ byteTable1[(secondHalf >>> 8) & 0xFF]
>                             ^ byteTable2[(secondHalf >>> 16) & 0xFF]
>                             ^ byteTable3[secondHalf >>> 24]
>                             ^ byteTable4[crc & 0xFF]
>                             ^ byteTable5[(crc >>> 8) & 0xFF]
>                             ^ byteTable6[(crc >>> 16) & 0xFF]
>                             ^ byteTable7[crc >>> 24];
>                 }
>             }
>
>             if (ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN) {
>                 crc = Integer.reverseBytes(crc);
>             }
>         }
>
>         // Tail
>         for (; adr < end; adr++) {
>             crc = (crc >>> 8)
>                     ^ byteTable[(crc ^ UNSAFE.getByte(adr)) & 0xFF];
>         }
>
>         return crc;
>     }
>
>
>>
>> Also, in updateBytes, the usage of 
>> Unsafe.ARRAY_INT_INDEX_SCALE/ARRAY_LONG_INDEX_SCALE to index a byte 
>> array sounds a little scary. To be ultra portable you could check 
>> that ARRAY_BYTE_INDEX_SCALE == 1 first and refuse to use Unsafe for 
>> byte arrays if it is not 1. Then use Integer.BYTES/Long.BYTES to 
>> manipulate 'offsets' instead. In updateDirectByteBuffer it would be 
>> more appropriate to use Integer.BYTES/Long.BYTES too.
> Good idea. Added a check in the initial if statement and it will get 
> automatically optimized away.
>
>>   225                         firstHalf = (int) (value & 0xFFFFFFFF);
>>   226                         secondHalf = (int) (value >>> 32);
>>   227                     } else { // ByteOrder.BIG_ENDIAN
>>   228                         firstHalf = (int) (value >>> 32);
>>   229                         secondHalf = (int) (value & 0xFFFFFFFF);
>>
>> firstHalf = (int) value; // this is equivalent for line 225
>> secondHalf = (int) value; // this is equivalent for line 229
> Done.
>
> Here is the latest webrev, 
> http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.03
>
> Cheers,
> Staffan




More information about the core-libs-dev mailing list