RFR: 8310843: Reimplement ByteArray and ByteArrayLittleEndian with Unsafe [v6]
Uwe Schindler
uschindler at openjdk.org
Thu Jun 29 08:42:56 UTC 2023
On Thu, 29 Jun 2023 07:59:42 GMT, Glavo <duke at openjdk.org> wrote:
> I understand the original reason for retaining it before, but this discussion is outdated for this PR.
>
> `Unsafe` does not provide `getFloatUnaligned`/`getDoubleUnaligned` and `putFloatUnaligned`/`putDoubleUnaligned`, so we must convert floating point numbers to integers here. This problem is no longer hidden behind `VarHanle`, so we have to face it.
>
> In addition, these comments are indeed wrong and may mislead users. I saw this mentioned in the previous discussion, but I don't know why the document was not cleaned up. I think it's time to clean it up.
I was involved in the original discussion. The problem that leads to the misunderstanding is the asymetry. It is only important to make sure that stuff writen to files needs to have normalized NaN. So basically, RandomAccessFile and others should use Float.toIntBits when writing and not raw int bits.
The other way round in reality does not matter at all, because when reading from a file the bits can be kept as is, because the JVM later knows how to handle them. There is also no 'Float.rawIntBitsToFloat()'. Actually behind the scenes the `Float.intBitsToFloat()` method already takes care to convert the int bits, so it works with both raw and normalized floats.
In my opinion, this PR makes sense, iff you want to use the class on early startup. Back at the time of original PR, using VarHandle (like Apache Lucene does) was the most elegant way to do this. It only affected RandomAccessFile and ObjectInput/OutputStream, so having a clean VarHandle implementation was perfectly fine and not critical for startup times.
I am fine with this, just make sure to remove outdated comments or correct them. The whole thing should be handled like: if you write floats/doubles make sure to give the option to call to normalize them or not. For writing to files this is a must.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14636#issuecomment-1612640215
More information about the core-libs-dev
mailing list