RFR JDK-6321472: Add CRC-32C API

Staffan Friberg staffan.friberg at oracle.com
Thu Nov 6 15:03:50 UTC 2014


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