[15?] RFR (S): 8249192: MonitorInfo stores raw oops across safepoints

David Holmes david.holmes at oracle.com
Thu Jul 23 12:05:53 UTC 2020


Hi Thomas,

Incrementals looks good to me.

Thanks,
David
-----

On 23/07/2020 7:39 pm, Thomas Schatzl wrote:
> Hi Dan and Serguei,
> 
>    thanks for your reviews.
> 
> On 22.07.20 19:04, Daniel D. Daugherty wrote:
>>> jdk15:
>>>
>>> http://cr.openjdk.java.net/~tschatzl/8249192/webrev.jdk15.2/ (full) 
>>
>> src/hotspot/share/prims/jvmtiEnvBase.cpp
>>      old L1029:       ResourceMark rm;
>>          It's not clear (to me anyway) why this ResourceMark is removed.
>>
>>          Update: I saw the discussion of "ResourceMark rm" in JDK15 
>> versus
>>          "ResourceMark rm(current_thread)" in JDK16, but that doesn't 
>> tell
>>          me why it was necessary to remove that ResourceMark.
> 
> The method that is guarded by this ResourceMark contains the necessary 
> ResourceMark itself, so I removed it.
> 
>>
>> src/hotspot/share/prims/stackwalk.cpp
>>      L291:     ResourceMark rm;
>>      L292:     HandleMark hm;
>>          Since there's a TRAPS parameter, these should be 'rm(THREAD)' 
>> and
>>          'hm(THREAD)'.
>>
>> src/hotspot/share/runtime/biasedLocking.cpp
>>      No comments.
>>
>> src/hotspot/share/runtime/deoptimization.cpp
>>      No comments.
>>
>> src/hotspot/share/runtime/vframe.cpp
>>      L461:   _lock  = lock;
>>          nit - extra space before '='.
>>
>> src/hotspot/share/runtime/vframe.hpp
>>      L32: #include "runtime/handles.inline.hpp"
>>          nit - new include is out of order; should be after frame.hpp.
>>
>> src/hotspot/share/runtime/vframeArray.cpp
>>      No comments.
>>
>> src/hotspot/share/runtime/vframe_hp.cpp
>>      Skipped - no changes.
>>
>> src/hotspot/share/services/threadService.cpp
>>      No comments.
>>
> 
> All fixed, and incorporating Serguei's changes in the other email as well.
> 
> jdk16:
> http://cr.openjdk.java.net/~tschatzl/8249192/webrev.3/ (full)
> http://cr.openjdk.java.net/~tschatzl/8249192/webrev.2_to_3/ (diff)
> 
> jdk15:
> http://cr.openjdk.java.net/~tschatzl/8249192/webrev.jdk15.3/ (full)
> http://cr.openjdk.java.net/~tschatzl/8249192/webrev.jdk15.2_to_3/ (diff)
> 
> Note that the jdk15 change will only go into 15.0.2 as discussion with 
> the release team showed that the change is too risky for earlier 
> releases. See the relevant CR comment for details.
> 
> Thanks,
>    Thomas
> 


More information about the serviceability-dev mailing list