RFR 8194085: Obsolete the deprecated SafepointSynchronize flags and remove related code

[harold seigel]

Hi Dan,

Thanks for reviewing this!  Please see comments in-line.


On 2/21/2018 2:33 PM, Daniel D. Daugherty wrote:
> On 2/21/18 1:31 PM, harold seigel wrote:
>> Hi,
>>
>> Please review this JDK-11 change to obsolete the 
>> SafepointSpinBeforeYield, DeferThrSuspendLoopCount, and 
>> DeferPollingPageLoopCount options and related code.  With this 
>> change, these options are still accepted on the command line but have 
>> no affect other than to generate these warning messages:
>>
>>    Java HotSpot(TM) 64-Bit Server VM warning: Ignoring option
>>    SafepointSpinBeforeYield; support was removed in 11.0
>>    Java HotSpot(TM) 64-Bit Server VM warning: Ignoring option
>>    DeferThrSuspendLoopCount; support was removed in 11.0
>>    Java HotSpot(TM) 64-Bit Server VM warning: Ignoring option
>>    DeferPollingPageLoopCount; support was removed in 11.0
>>
>> Open Webrev: 
>> http://cr.openjdk.java.net/~hseigel/bug_8194085/webrev/index.html
>
> src/hotspot/share/runtime/globals.hpp
>     No comments.
>
> src/hotspot/share/runtime/safepoint.hpp
>     L110 - Any particular reason for adding the extra blank line?
No particular reason.  I'll remove it.
>
> src/hotspot/share/runtime/safepoint.cpp
>     L81: int SafepointSynchronize::_defer_thr_suspend_loop_count = 4000;
>
>         In safepoint.hpp:
>
>             L211:   static void set_throughput_mode() {
>             L212:     _defer_thr_suspend_loop_count = 1;
>
>         And in arguments.cpp:
>
>             L2947:     } else if (match_option(option, 
> "-Xconcurrentio")) {
>             L2954: SafepointSynchronize::set_throughput_mode();
>
>         So before we had the ability to tune the DeferThrSuspendLoopCount
>         value when the "-Xconcurrentio" option was specified. Now we use
>         the setting of "1" when the "-Xconcurrentio" option is specified
>         with no way to override that value. Okay.
My understanding is that before we could use "-Xconcurrentio" to, among 
other things, set DeferThrSuspendLoopCount to 1. "-Xconcurrentio" could 
not be used to set DeferThrSuspendLoopCount to any other value.  You 
needed to use -X:DeferThrSuspendLoopCount=<value> to do that.  This 
change obsoletes DeferThrSuspendLoopCount but preserves the existing 
"-Xconcurrentio" functionality.

Thanks, Harold
>
>         So in this bug:
>
>             8191327 Deprecate DeferThrSuspendLoopCount
>
>         the option was deprecated, but there's no real explanation for
>         why this is okay. I understand the need to simplify the safepoint
>         code, but there isn't any rationale for why this won't impact
>         customers other than the fact that this option has been
>         advertised as:
>
>             product(intx, DeferThrSuspendLoopCount,     4000, \
>                     "(Unstable, Deprecated) "
>
>         Okay, I'm done mulling here. Since I've never heard of the
>         "-Xconcurrentio" option before today, I'm not going to stress
>         over the tuneablity of an unstable option when -Xconcurrentio"
>         is used.
>
> src/hotspot/share/runtime/arguments.cpp
>     No further comments.
>
> Thumbs up!
>
> Dan
>
>
>>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8194085
>>
>> The change was tested with Mach5 tiers 1 and 2 on all Mach5 platforms 
>> and tiers 3-5 in Linux-X64.
>>
>> Thanks, Harold
>