On Thu, 29 Jun 2023 07:59:42 GMT, Glavo <d...@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

Reply via email to