[lworld] RFR JDK-8207259: Ensure VarHandle compare and set/exchange work for value types

[Remi Forax]

Hi Mandy,
the use of Objects.equals to test two values is i believe wrong because a value type can have its method equals overriden but here you want to test the component wise equality bypassing the user defined equals.

and as a meta/spec comment, i do not think that using a lock is the right semantics, we should not allow compareAndSet on values, as a user if i use a CAS i want the platform to use a CAS (or equivalent) not a lock, it's important that the performance model of Java stay predictable for a user.
so we need a flag flattenIfAtomic which avoid flattening a field if the platform is not able to perform a CAS on the flattened value of that field. A volatile field is implicitly flattenIfAtomic, but because we can do a CAS without the field been marked volatile using VarHandles, we need a separated flag.

cheers,
Rémi

----- Mail original -----
> De: "mandy chung" <mandy.chung at oracle.com>
> À: "valhalla-dev" <valhalla-dev at openjdk.java.net>
> Envoyé: Mercredi 15 Août 2018 19:45:17
> Objet: [lworld] RFR JDK-8207259: Ensure VarHandle compare and set/exchange work for value types

> Webrev:
>    http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8207259/webrev.01
> 
> This patch fixes the compare and set/exchange to work properly for
> flattened, and non-flattened value types properly.  The new VarHandle
> access tests cover flattened, non-flattened value fields and arrays and
> VarHandle whose symbolic type is Object[] writing on a flattened array.
> 
> - Unsafe::compareAndSetObject and compareAndExchangeObject may be
>   called for writing to a non-flattened value field.  In that case,
>   it should perform the substitutability check (rather than identity
>   check).  We probably need some unsafe API to detect if the operands
>   are identityless.  For now, Unsafe::compareAndSetObject and
>   compareAndExchangeObject take an additional field type parameter
>   and determine if it's identityless based on the field type and
>   the expected object.
> 
> - refactor VarHandleObjects to split the support for flat value and
>   generate from X-VarHandle.java.template a new class VarHandleValues
>   at build time.   I think this is a cleaner way to handle values.
>   VarHandleObjects provides VarHandle support for references including
>   non-flattened value fields that uses Unsafe reference access e.g
>   getObject/putObject whereas VarHandleValues provides VarHandle
>   support for flattened value fields that uses Unsafe flat value access.
> 
>   (I'm working on a separate patch to rename xxxObject to xxxReference
>    to make the Unsafe API explicit).
> 
> - VarHandleObjects.Array has to handle Object[] as the symbolic type
>   but the array instance is a flattened value type that might have
>   the performance impact for VarHandle for reference types.  The
>   implementation tests if VarHandle symbolic type is Object[]
>   and the array object is flattened array and hence the impact
>   hopefully is limited but will wait for performance number to tell.
> 
> - The new VarHandleTestAccessPoint test crashes (SIGSEGV) when running
>   with compiler.  I will file a JBS issue separately.  So the test
>   for now will run only with -Xint.
> 
> Thanks
> Mandy