RFR(S): 8217658 baseline_cleanups from Async Monitor Deflation project

[David Holmes]

Hi Dan,

This all seems fine. Some comments below.

One suggested addition to your cleanup :)

./share/runtime/basicLock.cpp

// [RGV] The next line appears to do nothing!
   intptr_t dh = (intptr_t) displaced_header();
   dest->set_displaced_header(displaced_header());

On 24/01/2019 6:07 am, Daniel D. Daugherty wrote:
> Greetings,
> 
> I have a (S)mall patch extracted from the Async Monitor Deflation
> project that is ready for code review.
> 
> The short version of what this patch is about:
> 
>      This sub-task captures updates to the baseline code that are not
>      directly related to Async Monitor Deflation.
> 
> The details are in the bug report:
> 
>      JDK-8217658 baseline_cleanups from Async Monitor Deflation project
>      https://bugs.openjdk.java.net/browse/JDK-8217658
> 
> Here's the webrev:
> 
>      http://cr.openjdk.java.net/~dcubed/8217658-webrev/0-for-jdk13/

To quote your main semantic changes from the bug report:

> - add missing "else" to logical if-else-statement in
>   ObjectSynchronizer::get_lock_owner()

Okay - the two conditions are mutually exclusive.

> - guarantee that header() is NULL when
>   ObjectSynchronizer::omRelease() is called
> - Replace a set_owner(NULL) call with a set_header(NULL)
>   call in the "CASE: neutral" part of ObjectSynchronizer::inflate()
>   when the object->cas_set_mark() fails. This is a rare race. 

These seem okay but did get me thinking about the different roles of 
ObjectMonitor::Recycle() and ObjectMonitor::clear() and the individual 
clearing of fields! I find it all somewhat confusing - in particular why 
clear() asserts that _header != NULL ! Some explanatory comments on 
those methods would be appreciated at some point during this project.

Thanks,
David
-----


> 
> This patch along with the other two patches for Async Monitor Deflation
> project have been through Mach5 
> builds-tier1,hs-tier1,jdk-tier1,hs-tier2,hs-tier3
> testing and I'm currently running my stress testing kits on my Linux-X64
> and Solaris-X64 servers.
> 
> Thanks, in advance, for any questions, comments or suggestions.
> 
> Dan
>