RFR JDK-6321472: Add CRC-32C API
Joe Darcy
joe.darcy at oracle.com
Tue Oct 21 20:58:25 UTC 2014
On 10/21/2014 01:11 PM, Staffan Friberg wrote:
> Converted.
>
> http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.05
Conversion looks fine; thanks,
-Joe
>
> //Staffan
>
> On 10/21/2014 12:34 PM, Joe Darcy wrote:
>> Hi Staffan,
>>
>> If you are updating package.html, please also hg mv the file to be a
>> package-info.java file with the equivalent javadoc.
>>
>> Thanks,
>>
>> -Joe
>>
>> On 10/21/2014 11:49 AM, Staffan Friberg wrote:
>>> 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