RFR JDK-6321472: Add CRC-32C API
Staffan Friberg
staffan.friberg at oracle.com
Tue Oct 21 20:11:13 UTC 2014
Converted.
http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.05
//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