RFR (M): 8149159: Clean up Unsafe

Paul Sandoz paul.sandoz at oracle.com
Wed Mar 2 12:33:01 UTC 2016


> On 1 Mar 2016, at 17:58, Mikael Vidstedt <mikael.vidstedt at oracle.com> wrote:
> 
> 
> Thanks for all the great feedback/questions. Updated webrev(s) here:
> 
> http://cr.openjdk.java.net/~mikael/webrevs/8149159_unsafecleanup/hotspot/webrev.01.incr/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8149159_unsafecleanup/jdk/webrev.01.incr/webrev/
> 

+1


> Also, here are the changes after rebasing on top of hs-comp to include Aleksey's recent changes to Unsafe. There were some conflicts in unsafe.cpp where the new methods/functions were added, so I had to resolve that and apply the cleanup to them as well.
> 
> hotspot: http://cr.openjdk.java.net/~mikael/webrevs/8149159_unsafecleanup/hotspot/webrev.01/webrev/
> jdk: http://cr.openjdk.java.net/~mikael/webrevs/8149159_unsafecleanup/jdk/webrev.01/webrev/
> 
> 
> * Changes
> 
> Apart from resolving the conflicts in unsafe.cpp, this change include the following changes compared to webrev.00:
> 
> Claes - you are absolutely about right about the missing annotations, I have gone through and added the ForceInline annotation to all the s.m.Unsafe methods were it was missing. Thanks for catching that!
> 

IMO this is a nice to have and good to be consistent to avoid any missing anything; not strictly necessary for methods that are not marked as intrinsic in the “real” Unsafe (i assumed, perhaps incorrectly, that was the case for all such non-annotated methods).


> Chris - I chose to completely remove the UnsafeWrapper macro and its uses from unsafe.cpp. After all it's empty right now, and if somebody feels like adding it back it can simply be injected into the UNSAFE_ENTRY/UNSAFE_LEAF macros.
> 
> 
> * Testing
> 
> I've run both -testset hotspot and -testset core on this, and FWIW it's all green.
> 
> 
> * Comments/other
> 
> Stas - Wrt to the changes to the unsafe.cpp function names, I chose to rename the functions to include the suffix "0" to reflect the name change of the corresponding java (native) methods. While it's not strictly necessary it does feel just a tad clearer/more intuitive to me to have the names match.
> 
> John/Paul - I filed https://bugs.openjdk.java.net/browse/JDK-8150921 to cover the migration from single (long) addressing modes for the Unsafe getters/setters to the double-register (Object + long) equivalents.
> 

Great! For that we might require some careful analysis of generated code to ensure the double addressing accessor with a null base value generates the same code as the current single addressing accessor.

Paul.


More information about the jdk9-dev mailing list