RFR 8231264: Disable biased-locking and deprecate all flags related to biased-locking

[Doerr, Martin]

Hi Dan,

> As for the whole "too soon to deprecate" discussion: Deprecation is not
> making the code obsolete so this changeset is not taking anything away
> other than changing the default of UseBiasedLocking from true to false.
> There are things that have been deprecated since JDK8 and they still
> have not yet been made obsolete.

I think deprecating before publishing an evaluation or at least having a discussion is not appropriate.

> Deprecating biased locking is the proper way of saying that we (Oracle)
> and/or others think that biased locking should/will go away in a future
> release. Yes, there are locking experts outside of Oracle that have said
> that biased locking should go away, but I haven't gotten permission to
> quote the folks (yet)...

There should be consent on the direction of possibly removing it before communicating it the hard way.
However, switching it off for evaluation sounds feasible to me.
Seems like we have some homework, too.

Thanks, Patricio, for going the JEP way. I think changes with less impact have already been handled as JEP.

Best regards,
Martin


> -----Original Message-----
> From: hotspot-runtime-dev <hotspot-runtime-dev-
> bounces at openjdk.java.net> On Behalf Of Daniel D. Daugherty
> Sent: Dienstag, 19. November 2019 00:06
> To: Patricio Chilano <patricio.chilano.mateo at oracle.com>; hotspot-runtime-
> dev at openjdk.java.net
> Subject: Re: RFR 8231264: Disable biased-locking and deprecate all flags
> related to biased-locking
> 
> Hi Patricio,
> 
> On 11/15/19 9:15 PM, Patricio Chilano wrote:
> > Hi all,
> >
> > Could you review the following patch?
> >
> > JBS: https://bugs.openjdk.java.net/browse/JDK-8231264
> > Webrev: http://cr.openjdk.java.net/~pchilanomate/8231264/v01/webrev
> 
> src/hotspot/share/runtime/arguments.cpp
>      Is it too early to specify the obsolete_in and expired_in values?
>      They could be JDK_Version::undefined() so that all you are doing
>      is deprecation in this changeset.
> 
> src/hotspot/share/runtime/globals.hpp
>      No comments.
> 
> test/hotspot/gtest/oops/test_markWord.cpp
>      L96:     // Can't test this with biased locking disabled.
>          Perhaps (since the comment is inside the if-statement):
>               // This sub-test requires biased locking to be enabled.
> 
>      L11[135] - Why indent the pre-processor controls? Left most
>          column is generally the style used.
> 
>      L115:   // Same thread tries to lock it again.
>          This comment needs a rewrite. Perhaps:
>              // Lock the object using an ObjectLocker helper which
>              // will revoke the bias if we happened to use that
>              // mechanism above.
> 
>      L121:   // This is no longer biased, because ObjectLocker revokes
> the bias.
>          This comment needs a rewrite. Perhaps:
>              // The object should be unlocked with no hashCode at
>              // this point (ObjectLocker dtr has run).
> 
> test/jdk/jdk/jfr/event/runtime/TestBiasedLockRevocationEvents.java
>      No comments.
> 
> Thumbs up! My comments are mostly nits so I don't need to see a new
> webrev if you decide to make changes based on my suggestions.
> 
> As for the whole "too soon to deprecate" discussion: Deprecation is not
> making the code obsolete so this changeset is not taking anything away
> other than changing the default of UseBiasedLocking from true to false.
> There are things that have been deprecated since JDK8 and they still
> have not yet been made obsolete.
> 
> Deprecating biased locking is the proper way of saying that we (Oracle)
> and/or others think that biased locking should/will go away in a future
> release. Yes, there are locking experts outside of Oracle that have said
> that biased locking should go away, but I haven't gotten permission to
> quote the folks (yet)...
> 
> Deprecation is not final. Features can be un-deprecated if some
> relevant facts and/or info changes the previous conclusion.
> 
> Dan
> 
> 
> 
> >
> > Biased locking will be disabled by default and all related flags will
> > be deprecated. Performance gains seen when the feature was introduced
> > in the VM are less clear today with modern Java code/processors.
> > Detailed rationale behind the change is included on the description of
> > the bug.
> >
> > I modified test gtest/oops/test_markWord.cpp so that it still
> > exercises other cases of markword printing.
> >
> > Tested with mach5, tiers1-6 on all platforms (Linux, macOS, Windows
> > and Solaris).
> >
> > Thanks,
> > Patricio
> >
> >