On Wed, 4 Jan 2023 12:20:28 GMT, Per Minborg <pminb...@openjdk.org> wrote:

>> Currently, `java.io.Bits` is using explicit logic to read/write various 
>> primitive types to/from byte arrays. Switching to the use of `VarHandle` 
>> access would provide better performance and less code. 
>> 
>> Also, using a standard API for these conversions means future `VarHandle` 
>> improvements will benefit `Bits` too. 
>> 
>> Improvements in `Bits` will propagate to `ObjectInputStream`, 
>> `ObjectOutputStream` and `RandomAccessFile`.
>> 
>> Initial benchmarks and performance discussions can be found here: 
>> https://github.com/openjdk/panama-foreign/pull/762
>
> Per Minborg has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add special test values for double and float

Hi,
i checked you new test, but this one does not help with the problem of 
`Double#doubleToLongBits()` (as defined in the spec of `RandomAccessFile`!) vs 
the VarHandle, which just puts the raw bits without normalization (see 
javadocs). Your test only checks with the canonic NaN, which should already be 
"normalized" so it round trips. Basically it has to be made sure, that `Bits` 
save any variant of double NaN as `0x7ff8000000000000L` (similar for float).

My suggestion is the following. It is unsymmetric, but "correct" accoring to 
spec:
- the read methods are perfectly fine, so they directly read the double/float 
through its corresüponding varhandle.
- the write method should be reverted to the original version before the patch 
(they used floatBitsToInt/doubleBitsToLong and delegated to the putLong/putInt 
- which now uses varhandle). This makes sure that we convert the double/float 
to its normalized form and then write it as a long.

We may add a test to check if the NaN produced by several division operations 
produce the normalized value in the byte array. Iam not sure what the best way 
to get de-normalized NaNs. Maybe theres a test around.

-------------

PR: https://git.openjdk.org/jdk/pull/11840

Reply via email to