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

[David Holmes]

Hi Harold,

On 22/02/2018 4:31 AM, 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

Just for the record these were once useful tuning values for 
investigating safepoint performance issues in the field.

> Open Webrev: 
> http://cr.openjdk.java.net/~hseigel/bug_8194085/webrev/index.html

This one is a bit messier than I had envisaged: -Xconcurrentio ?? Wow! 
:) Definitely overlooked that one. It should be deprecated in 11 for 
certain.

I have one problem with a change in safepoint.cpp, the removal of this:

  312           if (SafepointMechanism::uses_global_page_poll() && 
int(iterations) == DeferPollingPageLoopCount) {
  313             guarantee (PageArmed == 0, "invariant") ;
  314             PageArmed = 1 ;
  315             os::make_polling_page_unreadable();
  316           }

This block triggers a guarantee failure if we fail to reach a safepoint 
and the number of iterations overflows to reach -1 (the value of 
DeferPollingPageLoopCount). That is the only thing that stops the VM 
from just hanging when we have an issue reaching safepoints (like all of 
the "counted loop" problems!) and is very valuable in clearly 
identifying the problem. So I strongly recommend that this is restored 
with an explicit check against -1.

---

src/hotspot/share/runtime/safepoint.hpp

+   static void set_throughput_mode() {
+     _defer_thr_suspend_loop_count = 1;
+   }

First can you add a comment before this that it is only used for 
-Xconcurrentio support.

Second, the name seems odd - throughput mode? How about 
set_concurrentio_mode() ? Or even just set_defer_thr_suspend_loop_count() ?

Thanks,
David
-----

> 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