8139891: Prepare Unsafe for true encapsulation (original) (raw)

David Holmes david.holmes at oracle.com
Mon Oct 26 01:41:23 UTC 2015

Hi Chris,

Updated webrev: http://cr.openjdk.java.net/~chegar/unsafephase1.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/unsafephase1/ 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:: isunsafealias is not a useful API point. I suggest making that function be a local, static helper for initintrinsicid. 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 librarycall.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::internalUnsafeklass is future-proof in this respect. Updated webrev: http://cr.openjdk.java.net/~chegar/unsafephase1.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); }

More information about the core-libs-dev mailing list