RFR(S): 8150921: Update Unsafe getters/setters to use double-register variants

[Aleksey Shipilev]

On 05/03/2016 09:43 PM, Mikael Vidstedt wrote:
> On 5/3/2016 8:15 AM, Aleksey Shipilev wrote:
>> *) Why do we Handle-ize incoming object in CompareAnd*? Is this because
>> we may acquire a mutex, and the object may be gone? Either way, I see
>> you have dropped Handle from Unsafe_CompareAndSwapLong, but kept it in
>> Unsafe_CompareAndExchangeLong -- why?
>>
>> *) Ditto for old {Get|Set}LongVolatile -- it used to Handle-ize before
>> acquiring a mutex -- now we don't do that in MemoryAccess.
> 
> My bad. As you say resolving the object should happen after the mutex
> has been acquired (see long comment starting on line 377 for the
> background). I have added back the Handle in CompareAndSwapLong and
> moved the addr() call in the mutex methods of MemoryAccess. Thank you
> for catching that!

Right. We have two related problems:

 a) the code in Unsafe_{Get|Set}LongVolatile computes the address
*before* acquiring the mutex, contrary to what comment suggests for
safety; this is fixed by your patch -- MemoryAccess handles that;

 b) the code in Unsafe_CompareAnd{Exchange,Swap}Long computes the
address *before* acquiring the mutex, and this is still problematic; I
wonder if you want to soak in CAS/CAE into MemoryAccess too?

I actually wonder why those accessors (especially non-long) are
UNSAFE_ENTRY, not UNSAFE_LEAF, if "we can treat those as leaf routines"...

Another question to pile on: why only LongVolatile is treated specially,
but not DoubleVolatile too? Atomicity requirements are the same for both
long and double.

>> vmSymbols.cpp:
>>
>> *) {get|put}Address_raw intrinsics used to be conditionalized under
>> UseUnalignedAccesses. Baffles me why, because the old native code in
>> Unsafe_{Get|Set}NativeAddress/unsafe.cpp would seem to fail with
>> misaligned addresses. I wonder if the intent was to never fail, and then
>> we should really use Unsafe.{get|put}Unaligned in new Unsafe.java
>> {get|set}Address to dodge misalign faults?
> 
> I also failed to see what the conditional intrinsification is trying to
> achieve given that the native code does nothing special for the
> unaligned case. Unless I'm missing something the proposed change makes
> it no worse, and will potentially improve performance a tad on the
> pickier platforms. If anything we may want to consider introducing
> methods for getAddressUnaligned and putAddressUnaligned as a separate
> change.

Yes, there is no advertised behavior for misaligned {get|put}Address.
Only the intrinsic filters trip you off. I agree we can move on from there.

> jdk:
> http://cr.openjdk.java.net/~mikael/webrevs/8150921/webrev.02/jdk/webrev/
> hotspot:
> http://cr.openjdk.java.net/~mikael/webrevs/8150921/webrev.02/hotspot/webrev/

Thanks,
-Aleksey