RFR JDK-6321472: Add CRC-32C API

Staffan Friberg staffan.friberg at oracle.com
Thu Nov 20 12:45:12 UTC 2014


Hi,

Anyone who can be the second Reviewer?

Thanks,
Staffan

On 11/06/2014 04:03 PM, Staffan Friberg wrote:
> Hi Andrej,
>
> Thanks for your comments. New webrev, 
> http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.08
>
> Indeed more common :)
>
> jdk/src/java.base/share$ grep -R "private final static" *|wc -l
> 282
> jdk/src/java.base/share$ grep -R "private static final" *|wc -l
> 3274
>
> //Staffan
>
> On 11/06/2014 12:57 PM, Andrej Golovnin wrote:
>> Hi Staffan,
>>
>> I'm not a reviewer but I have small remarks to the code style:
>>
>> - the instance variable "crc" is declared bevor the class variables. 
>> I would move it closer to the constructor.
>>
>> - documentation comments should be used for the fields "crc" and 
>> "CRC32C_POLY", e.g. instead of
>>
>>      // Calculated CRC-32C value
>>      private int crc = 0xFFFFFFFF;
>>
>> use this:
>>
>>      /**
>>       * Calculated CRC-32C value
>>       */
>>      private int crc = 0xFFFFFFFF;
>>
>>
>> - the description of the implemented algorithm should use block 
>> comments and should be moved to the top of the class, e.g.:
>>
>>  public final class CRC32C implements Checksum {
>>
>>      /*
>>       * This CRC-32C implementation uses the 'slicing-by-8' algorithm 
>> described
>>       * in the paper "A Systematic Approach to Building High Performance
>>       * Software-Based CRC Generators" by Michael E. Kounavis and 
>> Frank L. Berry,
>>       * Intel Research and Development
>>       */
>>
>>   ...
>>
>> In other case I get the filling that the Usafe class implements the 
>> algorithm. :-)
>>
>> - why some of the fields declared as "private transient final 
>> static"? I would say "transient" is not needed here. Or am I missing 
>> something? And I would use "private static final". I think this is 
>> the preferred way in the JDK to declare a constant.
>>
>>   61     private transient final static Unsafe UNSAFE = 
>> Unsafe.getUnsafe();
>>   62
>>   63     // Lookup tables
>>   64     private transient final static int[][] byteTables = new 
>> int[8][256];
>>   65     private transient final static int[] byteTable;
>>   66     private transient final static int[] byteTable0 = 
>> byteTables[0];
>>
>>
>> Best regards,
>> Andrej Golovnin
>




More information about the core-libs-dev mailing list