RFR (M): 8149159: Clean up Unsafe

[John Rose]

This is good.  Reviewed, except for any further discussion (if needed) on extra checks in copyMemory.

The copyMemory intrinsic gets a little extra error checking, since copyMemory0 is the intrinsic, but cannot be accessed without the copyMemoryChecks.  (I see it's tested by CopyMemory.testNegative.)  It should not be a performance problem, except perhaps for very short arrays.  If there's a problem we can consider putting the @HSIC annotation on the wrapper (in which case the testNegative will also have to go away).  I'm *not* in favor of any systematic upgrade of argument testing on the Unsafe API.  If you sign up for Unsafe coding, you check your arguments yourself, or take the consequences, as your new Javadoc so eloquently states.

This could be a separate bug, but these one-address access methods are totally obsolete:

     @ForceInline
     public float getFloat(long address) {
         return theInternalUnsafe.getFloat(address);
     }

They should be recoded in terms of their more "modern" two-address equivalents:

     @ForceInline
     public float getFloat(long address) {
         return theInternalUnsafe.getFloat(null, address);
     }

And then (soon please?) we can remove them from the "real" Unsafe and from the vmIntrinsics list, and nuke all the DEFINE_GETSETNATIVE entry points in unsafe.cpp.  (…And their place shall know them no more.)

BTW, for Panama we want the two-address versions for loading and storing machine
words:  getAddress(oop,long) and putAddress(oop,long,long), but not the old one-address versions.
(This will allow us to work with native data structures both on-heap and off-heap.)

— John

On Feb 18, 2016, at 11:22 AM, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
> 
> 
> Please review the following change which does some relatively significant cleaning up of the Unsafe implementation.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8149159
> Webrev (hotspot): http://cr.openjdk.java.net/~mikael/webrevs/8149159_unsafecleanup/hotspot/webrev.00/webrev/
> Webrev (jdk): http://cr.openjdk.java.net/~mikael/webrevs/8149159_unsafecleanup/jdk/webrev.00/webrev/
> 
> Summary:
> 
> * To avoid code duplication sun.misc.Unsafe now delegates all work to jdk.internal.misc.Unsafe. This also means that the VM - and unsafe.cpp specifically - no longer needs to know or care about s.m.Unsafe.
> * The s.m.Unsafe delegation methods have all been decorated with @ForceInline to minimize the risk of performance regressions, though it is highly likely that they will be inlined even without the annotations.
> * The documentation has been updated to reflect that it is the responsibility of the user of Unsafe to make sure arguments are valid.
> * The argument checking has, to the extent possible, been moved from unsafe.cpp up to Java to simplify the native code and allow the JIT to optimize it.
> * Some of the argument checks have been relaxed. For example, the recently introduced U.copySwapMemory does not check for null pointers anymore. See docs for j.i.m.U.checkPointer for the complete reasoning behind this. Note that the Unsafe methods today, apart from U.copySwapMemory, do not perform the NULL related check(s).
> * A test was added for j.i.m.U.copyMemory, based on U.copySwapMemory. Feel free to point out that I should merge them (because I should).
> 
> Also, unsafe.cpp was cleaned up rather dramatically. Some specific highlights:
> 
> * Unsafe_ functions are now declared static, as are the other unsafe.cpp local functions.
> * Created unsafe.hpp and moved some functions used in other parts of the VM to that. Removed some "extern" function declarations (extern is bad, kittens die when extern is (over-)used).
> * Remove scary looking comment about UNSAFE_LEAF not being possible to use - there's nothing special about it, it's just a JVM_LEAF.
> * Used UNSAFE_LEAF for a few simple leaf methods
> * Added helpful braces around UNSAFE_ENTRY/UNSAFE_END to help auto-indent
> * Removed unused Unsafe_<...>##140 functions/macros
> * Updated macro argument names to be consistent throughout unsafe.cpp macro definitions
> * Replaced some checks with asserts - as per above the checks are now performed in j.i.m.Unsafe instead.
> * Removed all the s.m.Unsafe related code
> 
> 
> Testing:
> 
> * jtreg: hotspot_jprt group, jdk/internal
> * JPRT: hotspot testset
> * Perf: JMH unsafe-bench.jar (no significant changes)
> 
> I'm taking suggestions on additional things to test.
> 
> Cheers,
> Mikael
>