Also, ede
Unsafe.ADDRESS_SIZE == 4
That doesn't imply 32bit systems as with less than 32GiB (on 64bit) the
default is using compressed options and the address size is still only
4bytes.
I usually use something like this (in user space code) to detect the
architecture
private static final boolean is64Bit;
static{
final String p = java.security.AccessController.doPrivileged(new
PrivilegedAction<String>() {
@Override
public String run() {
return System.getProperty("os.arch",
"x64")+System.getProperty("sun.arch.data.model"", ");
}
});
is64Bit = p.indexOf("64")>=0;
};
Stanimir
On Fri, Oct 17, 2014 at 9:58 PM, Staffan Friberg <staffan.frib...@oracle.com
> wrote:
> On 10/17/2014 04:05 AM, Alan Bateman wrote:
>
>> On 17/10/2014 02:42, Staffan Friberg wrote:
>>
>>> Hi,
>>>
>>> This RFE adds a CRC-32C class. It implements Checksum so it will have
>>> the same API CRC-32, but use a different polynomial when calculating the
>>> CRC checksum.
>>>
>>> CRC-32C implementation uses slicing-by-8 to achieve high performance
>>> when calculating the CRC value.
>>>
>>> A part from adding the new class, java.util.zip.CRC32C, I have also
>>> added two default methods to Checksum. These are methods that were added to
>>> Adler32 and CRC32 in JDK 8 but before default methods were added, which was
>>> why they were only added to the implementors and not the interface.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6321472
>>> Webrev: http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.00
>>>
>>> I looked over the javadoc, I haven't found time to study the
>> implementation in CRC32C closely yet. Hopefully Sherman will be able to
>> review as he I think he has prototyped several CRC32C implementations.
>>
>> On Checksum#update(ByteBuffer) then a suggestion for:
>> "The checksum is updated using buffer.remaining, starting a
>> buffer.position"
>> is to replace it with:
>> "The checksum is updated with the remaining bytes in the buffer,
>> starting at the buffer's position."
>>
>
> Yes that reads much better. Updated CRC32 and Adler32 as well since they
> have the same text.
>
>
>> In the @implNote then I wonder if it would be better to just leave out
>> the note about when the invariants are broken, we don't do that in other
>> places where breakage this is detected. Also I wonder if the note about
>> "For best performance, ..." should be an @apiNote.
>>
> Done, removed the assert comment and changed the performance note to an
> @apiNote.
>
>
>> Should CRC32C be final unless we have a good reason for it not to be
>> final?
>>
> I simply followed what was used for CRC32 and Adler32, but I don't see a
> reason for not making it final. Guess it is too late to make those two
> classes final though?
>
>
>> In CRC32C then I assume you don't need the <p>/<p> for the first
>> statement. The @see Checksum might not be too interesting here given that
>> it will be linked to anyway by way of implement Checksum.
>>
>
> Removed @see in all three classes.
>
>
>> I think it's more usual to list the @param tags before the @throws.
>>
> I removed that @param tags as they are already described in the Checksum
> interface and will be picked up from there by Javadoc. Will do the same in
> CRC32 and Adler32 as well.
>
>
>> For the bounds check then you might want to look at the wording in other
>> areas (in java.lang or java.io for example) to get this better wording
>> (as off+len can overflow).
>>
> Done, I update CRC32 and Adler32 as well to make keep them as similar as
> possible.
>
>
>> A minor comment on CRC32C is that you might want to keep the line lengths
>> consistent with the rest of the code. I only mention is for future
>> side-by-side reviews to make it a bit easier and avoid too much horizontal
>> scrolling.
>>
> Done, lines are now <80 or very close in a few cases where breaking them
> made the code harder to read.
>
>>
>> -Alan
>>
>
> Here is a new webrev with the updates from Alan's and Peter's suggestions.
> http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.01
>
> Thanks,
> Staffan
>
>
