RFR(S/M): 8246476: remove AsyncDeflateIdleMonitors option and the safepoint based deflation mechanism

[coleen.phillimore at oracle.com]

http://cr.openjdk.java.net/~dcubed/8246476-webrev/0_for_jdk16/src/hotspot/share/runtime/objectMonitor.inline.hpp.udiff.html
http://cr.openjdk.java.net/~dcubed/8246476-webrev/0_for_jdk16/src/hotspot/share/runtime/synchronizer.cpp.udiff.html

+#ifdef ASSERT
    void* prev = Atomic::load(&_owner);
- ADIM_guarantee(prev == old_value, "unexpected prev owner=" INTPTR_FORMAT
+#endif
+ assert(prev == old_value, "unexpected prev owner=" INTPTR_FORMAT
                   ", expected=" INTPTR_FORMAT, p2i(prev), p2i(old_value));

Just a nit but these patterns look really strange.  Can you put the 
#endif on the other side of the assert?

http://cr.openjdk.java.net/~dcubed/8246476-webrev/0_for_jdk16/src/hotspot/share/runtime/safepoint.cpp.udiff.html

if 
(_subtasks.try_claim_task(SafepointSynchronize::SAFEPOINT_CLEANUP_DEFLATE_MONITORS)) 
{ ... + ObjectSynchronizer::do_safepoint_work();


Why do we still have this to trigger the ServiceThread when we're going 
to only wait GuaranteedSafepointInterval before checking for monitor 
deflation anyway?  Why have this in a safepoint cleanup task?

The code deletion is really nice.

Thanks,
Coleen


On 7/13/20 12:08 PM, Daniel D. Daugherty wrote:
> On 7/13/20 12:05 PM, Patricio Chilano wrote:
>> Hi Dan,
>>
>> Changes look good to me!
>
> Thanks!
>
>
>> In synchronizer.cpp we have this comment about ObjectMonitor lifecycle:
>>
>> // Inflation unlinks monitors from om_list_globals._free_list or a 
>> per-thread
>> // free list and associates them with objects. Deflation -- which 
>> occurs at
>> // STW-time or asynchronously -- disassociates idle monitors from 
>> objects.
>> // Such scavenged monitors are returned to the 
>> om_list_globals._free_list.
>>
>> With all the older code removed, are there still cases where we do 
>> deflations at safepoint?
>
> Good catch! I need to adjust that comment. I'll look for others also.
>
> Dan
>
>
>>
>> Thanks!
>> Patricio
>> On 7/13/20 10:47 AM, Daniel D. Daugherty wrote:
>>> Hi David,
>>>
>>> Thanks for the review!
>>>
>>> I need a second review folks... any takers?
>>>
>>> Dan
>>>
>>>
>>> On 7/12/20 10:57 PM, David Holmes wrote:
>>>> Hi Dan,
>>>>
>>>> This all looks good to me.
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>> On 8/07/2020 5:51 pm, David Holmes wrote:
>>>>> On 8/07/2020 7:21 am, Daniel D. Daugherty wrote:
>>>>>> Ping! Any takers??? Code deletion should be really appealing here!!
>>>>>
>>>>> Sorry Dan didn't get to it before vacation. But if you can wait 
>>>>> till Monday ...
>>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>> Dan
>>>>>>
>>>>>>
>>>>>> On 7/6/20 12:35 PM, Daniel D. Daugherty wrote:
>>>>>>> Greetings,
>>>>>>>
>>>>>>> It's time to remove the AsyncDeflateIdleMonitors option from 
>>>>>>> JDK16. We can
>>>>>>> also get rid of the safepoint based deflation mechanism since 
>>>>>>> turning off
>>>>>>> async deflation (-XX:-AsyncDeflateIdleMonitors) was the only way 
>>>>>>> left to
>>>>>>> use it.
>>>>>>>
>>>>>>> This is marked as an "S/M" review because the number of 
>>>>>>> touched/deleted
>>>>>>> lines makes it a Medium review, but the number of 
>>>>>>> touched/changed lines
>>>>>>> (outside of the deletions) makes it a Small review. It's 
>>>>>>> actually a pretty
>>>>>>> fast read... :-)
>>>>>>>
>>>>>>> Here's the bug ID:
>>>>>>>
>>>>>>>     JDK-8246476 remove AsyncDeflateIdleMonitors option and the 
>>>>>>> safepoint
>>>>>>>                 based deflation mechanism
>>>>>>>     https://bugs.openjdk.java.net/browse/JDK-8246476
>>>>>>>
>>>>>>> Here's the webrev URL:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~dcubed/8246476-webrev/0_for_jdk16/
>>>>>>>
>>>>>>> The webrev is baselined on Thomas S's fix for 8248650 which is 
>>>>>>> jdk-16+4
>>>>>>> plus a dozen or so changesets.
>>>>>>>
>>>>>>> This change has been tested with Mach5 Tier[1-3],4,5,6,7,8 and 
>>>>>>> there are
>>>>>>> no regressions (and very few known failures). My inflation 
>>>>>>> stress testing
>>>>>>> is still in process. I had to restart that testing after a 
>>>>>>> thunderstorm
>>>>>>> related power failure took down my servers in Florida. Sigh...
>>>>>>>
>>>>>>> Thanks, in advance, for any comments, questions, or suggestions.
>>>>>>>
>>>>>>> Dan
>>>>>>
>>>
>>
>