RFR(M): 8210848: Obsolete SyncKnobs

[coleen.phillimore at oracle.com]

This looks like a nice cleanup!

http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-full/open/webrev/src/hotspot/share/runtime/synchronizer.cpp.frames.html

verifyInUse can be removed also.

Also, the verification removed at line 1830 seems like it could be 
useful.  Can you make it an assert instead?  Sorry this might invalidate 
your testing.

http://cr.openjdk.java.net/~mikael/webrevs/8210848/webrev.00/syncknobs-full/open/webrev/src/hotspot/share/runtime/objectMonitor.cpp.frames.html

There are still some variables called Knob_Something that should 
probably be renamed to something meaningful, but could be done in a 
future RFE and not make this change larger.

That's all I saw.  Looks good!
Coleen

On 9/18/18 3:03 AM, 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
>