RFR JDK-6321472: Add CRC-32C API

joe darcy joe.darcy at oracle.com
Thu Nov 20 20:34:29 UTC 2014


Hello,

Core libs changes (at this stage of JDK 9) only require a single reviewer.

Cheers,

-Joe

On 11/20/2014 4:45 AM, Staffan Friberg wrote:
> 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