RFR JDK-6321472: Add CRC-32C API

Staffan Friberg staffan.friberg at oracle.com
Thu Oct 23 00:11:48 UTC 2014


Just realized that in the Checksum default case we will actually end up 
there for Direct buffers.

//Staffan

On 10/22/2014 05:06 PM, Staffan Friberg wrote:
> Hi,
>
> I was thinking about this earlier when I started writing the patch and 
> then I forgot about it again. I haven't been able to figure out when 
> the code will be executed. ByteBuffer is implemented in such a way  
> that only the JDK can extend it and as far as I can tell you can only 
> create 3 types of ByteBuffers (Direct, Mapped and Heap), all of which 
> will be handled by the more efficient calls above.
>
> That said just to make the code a bit safer from OOM it is probably 
> best to update the default method and all current implementations 
> which all use the same pattern.
>
> A reasonable solution should be the following code
>
>             byte[] b = new byte[(buffer.remaining() < 4096)
>                     ? buffer.remaining() : 4096];
>             while (buffer.hasRemaining()) {
>                 int length = (buffer.remaining() < b.length)
>                         ? buffer.remaining() : b.length;
>                 buffer.get(b, 0, length);
>                 update(b, 0, length);
>             }
>
> Xueming, do you have any further comment?
>
> Regards,
> Staffan
>
> On 10/22/2014 03:04 PM, Stanimir Simeonoff wrote:
>>
>>
>> On Thu, Oct 23, 2014 at 12:10 AM, Bernd Eckenfels 
>> <ecki at zusammenkunft.net <mailto:ecki at zusammenkunft.net>> wrote:
>>
>>     Hello,
>>
>>     just a question in the default impl:
>>
>>     +        } else {
>>     +            byte[] b = new byte[rem];
>>     +            buffer.get(b);
>>     +            update(b, 0, b.length);
>>     +        }
>>
>>     would it be a good idea to actually put a ceiling on the size of the
>>     array which is processed at once?
>> This is an excellent catch.
>> Should not be too large, probably 4k or so.
>>
>> Stanimir
>>
>>
>>      Am Tue, 21 Oct 2014 10:28:50 -0700
>>     schrieb Staffan Friberg <staffan.friberg at oracle.com
>>     <mailto:staffan.friberg at oracle.com>>:
>>
>>     > 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
>> <http://cr.openjdk.java.net/%7Esfriberg/JDK-6321472/webrev.03>
>>     >
>>     > Cheers,
>>     > Staffan
>>
>>
>




More information about the core-libs-dev mailing list