8139891: Prepare Unsafe for true encapsulation

[David Holmes]

Hi Chris,

> Updated webrev:
>    http://cr.openjdk.java.net/~chegar/unsafe_phase1.01/
>

This all looks fine to me.

> If you are in agreement, is it best to move this first step on and
> push it into hs-rt. I can then follow up with the additional steps:
> hotspot test updates in hs-rt, and the library changes in jdk9/dev,
> in parallel.

I would have expected all changes to stay together in the same forest 
(hs-rt), so I'm not clear what additional library changes in jdk9/dev 
you are referring to here.

Thanks,
David


On 24/10/2015 5:44 AM, Chris Hegarty wrote:
> Thanks for looking at this John. Comments inline…
>
> On 23 Oct 2015, at 02:06, John Rose <john.r.rose at oracle.com> wrote:
>
>> On Oct 22, 2015, at 7:28 AM, Chris Hegarty <chris.hegarty at oracle.com> wrote:
>>>
>>> To this end I've put together a webrev containing the changes required
>>> for step 1. It contains
>>>    - the intrinsic aliasing,
>>>    - the new internal Unsafe ( copy of sun.misc.Unsafe ),
>>>    - reverts sun.misc.Unsafe to, almost, its 1.8 API,
>>>    - minimal test and implementation updates, since there some usages
>>>      of unaligned access that is new in the 1.9.
>>>
>>> http://cr.openjdk.java.net/~chegar/unsafe_phase1/
>>
>> Looks good.
>
> Thanks.
>
>> The name jvm.internal.misc.Unsafe is begging to be split and renamed.
>> See http://www.inf.usi.ch/faculty/lanza/Downloads/Mast2015a.pdf for a good example
>> of how to put parts of Unsafe into logical buckets.
>> But that is a task for another day.
>
> Right. Hopefully, once we decouple from the sun.misc namespace future
> refactoring will be possible.
>
>> The VM changes will have to happen a second time when jvm.internal.misc.Unsafe
>> is refactored.  It is not clear how to pivot the current changes to prepare for that,
>> since the naming conventions in the JVM are very explicit about the concrete names
>> of intrinsics.
>
> Right. I just don’t want to make it, much, more difficult.
>
>> One suggestion:  I think the Method:: is_unsafe_alias is not a useful API point.
>> I suggest making that function be a local, static helper for init_intrinsic_id.
>> That way it doesn't have to show up in method.hpp, and that file won't have
>> to be edited when we refactor.
>
> Thanks. Update.
>
>> For a similar reason, I suggest changing the comments in library_call.cpp
>> so that they become more vague about the location of Unsafe:
>
> Good idea. I had this in a previous local version, but wasn’t sure
> which way to go. All references, in comments, to Unsafe are now
> more vague.
>
>> -// public native void sun.misc.Unsafe.putOrderedObject(Object o, long offset, Object x);
>> +-// public native void [sun|jdk.internal].misc.Unsafe.putOrderedObject(Object o, long offset, Object x);
>> ++// public native void Unsafe.putOrderedObject(Object o, long offset, Object x);
>>
>> That way the file won't have to be re-edited if we refactor.  Same point for
>> memnode.hpp and unsafe.cpp (and maybe other spots which say too much?).
>> It's already good in heapDumper.cpp.  In globals.hpp it's half good and half not-good.
>
> Done.
>
>> BTW, I think the name SystemDictionary::internal_Unsafe_klass is future-proof in this respect.
>
> Updated webrev:
>    http://cr.openjdk.java.net/~chegar/unsafe_phase1.01/
>
>> Thanks for taking this on!
>
> If you are in agreement, is it best to move this first step on and
> push it into hs-rt. I can then follow up with the additional steps:
> hotspot test updates in hs-rt, and the library changes in jdk9/dev,
> in parallel.
>
> -Chris.
>
>
>> — John
>>
>> P.S.  Just for the record, the native-heap-only memory accessors should not be intrinsics,
>> since they can be trivially derived from the normal two-argument accessors:
>>    -  public native byte    getByte(long address);
>>   +  public native byte    getByte(long address) {  return getByte(null, address);  }
>>
>