RFR (M): 8149159: Clean up Unsafe

[Stas Smirnov]

Hi Mikael,

overall changes look good, the only thing I did not quite get is the 
renaming of methods in hotspot, like Unsafe_CopyMemory -> 
Unsafe_CopyMemory0 with all the following, I counted three, changes, do 
we really need this, cause from what I see, you have only changed the 
implementation of this method, but left its signature and usage 
unchanged, though maybe I just missed something.

On 18/02/16 22:22, Mikael Vidstedt 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
>

-- 
Best regards,
Stanislav