Thanks for looking at this John. Comments inline… On 23 Oct 2015, at 02:06, John Rose <john.r.r...@oracle.com> wrote:
> On Oct 22, 2015, at 7:28 AM, Chris Hegarty <chris.hega...@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); } >
