RFR 8158039 VarHandle float/double field/array access should support CAS/set/add atomics

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Fri Jun 17 11:13:20 UTC 2016


Looks good.

Best regards,
Vladimir Ivanov

On 6/14/16 3:12 AM, Paul Sandoz wrote:
> Hi,
>
> Here is an updated webrev.
>
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8158039-float-double-field-array-cas.jdk/webrev/
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8158039-float-double-field-array-cas.hotspot/webrev/
>
> Changes
>
> - the Unsafe.getAndAdd for float/double operate in the bit-domain.
>
> - documentation added to the factory methods:
>
> 1221          * <p>
> 1222          * If the field type is {@code float} or {@code double} then numeric
> 1223          * and atomic update access modes compare values using their bitwise
> 1224          * representation (see {@link Float#floatToRawIntBits} and
> 1225          * {@link Double#doubleToRawLongBits}, respectively).
> 1226          * <p>
> 1227          * @apiNote
> 1228          * Bitwise comparison of {@code float} values or {@code double} values,
> 1229          * as performed by the numeric and atomic update access modes, differ
> 1230          * from the primitive {@code ==} operator and the {@link Float#equals}
> 1231          * and {@link Double#equals} methods, notably with respect to
> 1232          * {@code NaN}.  There are many possible NaN values that are considered
> 1233          * to be {@code NaN} in Java, although no IEEE 754 floating-point
> 1234          * operation provided by Java can distinguish between them.  As such
> 1235          * care should be taken when performing a compare and set or a compare
> 1236          * and exchange operation with NaN values since the operation may
> 1237          * unexpectedly fail.  Failure can occur if the expected or witness
> 1238          * value is a NaN value and it is transformed (perhaps in a platform
> 1239          * specific manner) into another NaN value, and thus has a different
> 1240          * bitwise representation (see {@link Float#intBitsToFloat} or
> 1241          * Double#longBitsToDouble for more details).
>
> (I also updated the documentation on array/ByteBuffer view methods but did not bother with the api note)
>
> Paul.
>
>> On 10 Jun 2016, at 15:39, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>>
>> Hi Joe,
>>
>> It’s my turn to catch up on email…
>>
>>
>>> On 6 Jun 2016, at 17:47, Joseph D. Darcy <Joe.Darcy at oracle.com> wrote:
>>>
>>> Hello,
>>>
>>> Catching up on email, there are several different notions on floating-point equality one might want to use. [1]
>>>
>>> One notion is the built-in "==" operator on float and double. This has the wrinkle of not defining an equivalence relation because of NaN values (not reflexive) and signed zeros (distinguishable values compare as equal).
>>>
>>> When I write floating-point tests, I'll use a notion of equality close to
>>>
>>>   Float.compare(x, y) == 0
>>>
>>> which means all NaN values are treated as the same and Float.compare(NaN, NaN) == 0 is true. This also distinguishes the signed zeros from each other.
>>>
>>> So comparing based on the non-raw bitwise conversion can give resonable semantics, at the cost of not preserving any payload stored in the NaN significand.
>>>
>>
>> And potentially a performance hit too [*].
>>
>> Pedantically since (NaN == NaN) is false, one could argue that it should not possible to CAS with an expected NaN value and/or a witness NaN value. Such behaviour might be considered BaNaNas :-) (sorry). I don’t think we can or should enforce that, but we should strongly advise against it and mention the pitfalls, and i will need to specify that the atomic ops perform bit-wise equality with suitable “as if” text.
>>
>> Paul.
>>
>> [*] For example, the double[] accepting Arrays.equals method used to compare each element as:
>>
>> 2762         for (int i=0; i<length; i++)
>> 2763             if (Double.doubleToLongBits(a[i])!=Double.doubleToLongBits(a2[i]))
>> 2764                 return false;
>>
>> In the interim of my adding vectorized mismatch support i changed that to:
>>
>> 3057         for (int i=0; i<length; i++) {
>> 3058             double v1 = a[i], v2 = a2[i];
>> 3059             if (Double.doubleToRawLongBits(v1) != Double.doubleToRawLongBits(v2))
>> 3060                 if (!Double.isNaN(v1) || !Double.isNaN(v2))
>> 3061                     return false;
>> 3062         }
>>
>> Which boosted the performance similar to that of comparing long[] (when no NaNs are present).
>>
>>
>>> HTH,
>>>
>>> -Joe
>>>
>>> [1] "Notions of Floating-Point Equality ," https://blogs.oracle.com/darcy/entry/notions_of_floating_point_equality
>>>
>>> On 6/2/2016 1:34 AM, Paul Sandoz wrote:
>>>>> On 2 Jun 2016, at 09:13, David Holmes <david.holmes at oracle.com> wrote:
>>>>>
>>>>> On 2/06/2016 1:24 AM, Paul Sandoz wrote:
>>>>>> (Please note this work is or will be covered with FC exception)
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Please review the VarHandles/Unsafe support for atomic ops on double/float fields/arrays.
>>>>> I think this is misguided. If the user wants CAS for float/double they can do the conversion to int/long in a context where they know that conversion makes sense and where they can deal with NaNs appropriately.
>>>>>
>>>> I would still like to support some clearly defined default behaviour if at all possible, since this is not easy to get right so we can add some value here. The user can use int/long if that behaviour is not suitable.
>>>>
>>>>
>>>>> At a minimum the fact this deals with raw bit patterns not semantic values needs to be exposed somehow. ie the handling of NaN needs to be explicit.
>>>>>
>>>> Yes, it’s multiple NaN values that collapse to a single NaN value.
>>>>
>>>> It would be good if our resident floating point expert Mr. Joe Darcy could chime in this respect.
>>>>
>>>> On Float.intBitsToFloat:
>>>>
>>>> * <p>Note that this method may not be able to return a
>>>> * {@code float} NaN with exactly same bit pattern as the
>>>> * {@code int} argument.  IEEE 754 distinguishes between two
>>>> * kinds of NaNs, quiet NaNs and <i>signaling NaNs</i>.  The
>>>> * differences between the two kinds of NaN are generally not
>>>> * visible in Java.  Arithmetic operations on signaling NaNs turn
>>>> * them into quiet NaNs with a different, but often similar, bit
>>>> * pattern.  However, on some processors merely copying a
>>>> * signaling NaN also performs that conversion.  In particular,
>>>> * copying a signaling NaN to return it to the calling method may
>>>> * perform this conversion.  So {@code intBitsToFloat} may
>>>> * not be able to return a {@code float} with a signaling NaN
>>>> * bit pattern.  Consequently, for some {@code int} values,
>>>> * {@code floatToRawIntBits(intBitsToFloat(start))} may
>>>> * <i>not</i> equal {@code start}.  Moreover, which
>>>> * particular bit patterns represent signaling NaNs is platform
>>>> * dependent; although all NaN bit patterns, quiet or signaling,
>>>> * must be in the NaN range identified above.
>>>>
>>>>
>>>>
>>>> I am concerned that the CAS loops could in some cases loop without termination:
>>>>
>>>> @ForceInline
>>>> public final float getAndAddFloat(Object o, long offset, float delta) {
>>>>    float v;
>>>>    do {
>>>>        v = getFloatVolatile(o, offset);
>>>>    } while (!weakCompareAndSwapFloatVolatile(o, offset, v, v + delta));
>>>>    return v;
>>>> }
>>>>
>>>>
>>>> I think this should be:
>>>>
>>>> @ForceInline
>>>> public final float getAndAddFloat(Object o, long offset, float delta) {
>>>>    int expectedBits;
>>>>    float v;
>>>>    do {
>>>>        expectedBits = getIntVolatile(o, offset);
>>>>        v = Float.intBitsToFloat(bits); // May not preserve a NaN value with the same bit pattern as expectedBits
>>>>    } while (!weakCompareAndSwapIntVolatile(o, offset, expectedBits, Float.floatToRawIntBits(v + delta)));
>>>>    return v;
>>>> }
>>>>
>>>> and likewise for the atomic get-and-set methods.
>>>>
>>>> Paul.
>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8158039-float-double-field-array-cas.jdk/webrev/ <http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8158039-float-double-field-array-cas.jdk/webrev/>
>>>>>> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8158039-float-double-field-array-cas.hotspot/webrev/ <http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8158039-float-double-field-array-cas.hotspot/webrev/>
>>>>>>
>>>>>> These patches are based on those of for sub-word CAS
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8157726 <https://bugs.openjdk.java.net/browse/JDK-8157726>
>>>>>> http://cr.openjdk.java.net/~shade/8157726/webrev.hs.03/ <http://cr.openjdk.java.net/~shade/8157726/webrev.hs.03/>
>>>>>> http://cr.openjdk.java.net/~shade/8157726/webrev.jdk.03/ <http://cr.openjdk.java.net/~shade/8157726/webrev.jdk.03/>
>>>>>>
>>>>>> And the two patches combined expand the support of fields/arrays.
>>>>>>
>>>>>>
>>>>>> New float/double CAS methods are added to Unsafe that defer to int/long equivalents. Then the other atomic methods are built from weak CAS loops.
>>>>>>
>>>>>> No changes are required to HotSpot, but changes are required to the Unsafe tests in the hotspot repository.
>>>>>>
>>>>>> In general the changes to VHs and tests are minimal since it is triggered from changes to the generating scripts that now include float/double into the CAS category.
>>>>>>
>>>>>> There are some minor specification changes and a CCC has been initiated.
>>>>>>
>>>>>> JPRT tests results show no relevant failures for hotspot and core test sets.
>>>>>>
>>>>>> Thanks,
>>>>>> Paul.
>>>>>>
>>>
>>
>


More information about the hotspot-dev mailing list