RFR(M): 8210848: Obsolete SyncKnobs

[Claes Redestad]

Great cleanup!

src/hotspot/share/runtime/objectMonitor.cpp:

   The DeferredInitialize method seems a bit silly now that it only 
guards the setting of three little Knobs using a CAS.. perhaps we would 
be better off wrapping these three in methods (spinLimit() { return 
os::is_MP() ? 5000 : 0; }), or perhaps we can guarantee that os::is_MP 
is initialized before, somehow? Either way I think we could profitably 
get rid of DeferredInitialize and InitDone.

/Claes

On 2018-09-18 09:03, Mikael Vidstedt wrote:
> Please review this change which obsoletes the SyncKnobs flag, and simplifies the code by removing the resulting unused code paths.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8210848
> Webrev: http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-full/open/webrev/ <http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-full/open/webrev/>
>
> * Background (from the bug)
>
> The experimental SyncKnobs flag can in theory be used to tune the behavior of the synchronization primitives. The flag was convenient when the various algorithms were compared a long time ago.
>
> In practice the only algorithm used and tested today are the default one. The SyncKnobs flag no longer serves the purpose it used to, and is "Unstable" (the documentation of the flag says so explicitly). It should be obsoleted and later removed.
>
> * About the change
>
> As mentioned above this change not only obsolete the SyncKnobs flag, but also removes the various sub-options/knobs and prunes the resulting dead code. Of course I’m happy to hear your thoughts on that. After having asked around it seems like the knobs have not been used the last decade or so, and I like to think that the synchronization code is complex enough without what is in that case effectively dead code.
>
> In order to make reviewing this slightly easier I have for your convenience (at least I hope it is) also created a bunch of smaller patches which stack, each removing one specific knob. Hopefully they’re more or less self-explanatory, but let me know if I can help clarify something:
>
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-00-base/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-01-Knob_ReportSettings/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-02-Knob_SpinBackOff/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-03-BackOffMask/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-04-Knob_ExitRelease/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-05-Knob_InlineNotify/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-06-Knob_Verbose/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-07-Knob_VerifyInUse/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-08-Knob_VerifyMatch/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-09-Knob_SpinBase/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-10-Knob_CASPenalty/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-11-Knob_OXPenalty/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-12-Knob_SpinSetSucc/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-13-Knob_SpinEarly/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-14-Knob_SuccEnabled/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-15-Knob_SuccRestrict/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-16-Knob_MaxSpinners/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-17-Knob_SpinAfterFutile/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-18-Knob_OState/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-19-Knob_UsePause/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-20-Knob_ExitPolicy/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-21-Knob_ResetEvent/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-22-Knob_FastHSSEC/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-23-Knob_MoveNotifyee/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-24-Knob_QMode/open/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-25-Obsolete/open/webrev/
>
> * Testing
>
> A slightly earlier version of this patch passed the standard tier1-3 testing. I will run additional testing after I get some feedback.
>
> Cheers,
> Mikael
>