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

Sun Nov 24 23:51:44 UTC 2019

On 23/11/2019 3:22 am, Erik Österlund wrote:
> Hi Martin,
> 
> Yeah, sorry it wasn’t communicated from the start. When this was discussed internally there were a whole bunch of factors considered. But loom interactions was really a primary concern in the discussions, and I was surprised to find it has yet to be mentioned in this thread!

Loom is not the primary motivation for getting rid of the complex and 
invasive biased-locking code. Loom is a factor to consider as I've 
raised in a number of discussions, but it seems not in this public one - 
apologies on that - removing BL _might_ make it easier for Loom. The 
reality is that we don't know what impact BL has on Loom. BL is only one 
part of the native locking process that Loom currently cannot support. 
We won't know the impact of BL until we try to support native monitors. 
It may be that removing BL whilst retaining the rest of the existing 
native code for ObjectMonitor use makes a huge simplification to the 
effort; or it may be lost in the noise. Or if we remove all native code 
(seems highly unlikely) then whether or not BL was removed earlier is moot.

Biased-locking is a very old optimization for uncontended locking, based 
on a time when there was heavy single-threaded use of synchronized data 
structures, and where actual lock/unlock atomic operations were very 
expensive. It is very complex and highly intrusive code. Every time "we" 
have had to make changes to object monitor support, or safepoint 
support, we have had to deal with the added complexity that 
biased-locking introduced and "we" have asked ourselves many times 
whether "we" can just get rid of this old optimization. And as I posted 
in one of my earliest responses to Aleksey those discussions finally 
reached the point where "we" decided to propose performing this removal. 
But step one is to ascertain the impact so the initial proposal is to 
disable biased-locking by default and deprecate it so that the change is 
much more visible.

David
-----

> Hope this makes a bit more sense now!
> 
> Thanks,
> /Erik
> 
>> On 22 Nov 2019, at 17:29, Doerr, Martin <martin.doerr at sap.com> wrote:
>>
>> Hi all,
>>
>> the only feasible argument for considering biased locking removal I've read so far is the one from Erik.
>> (Thanks, Erik!)
>>
>> This should have been on the table before proposing deprecation.
>>
>> If the plan is to deprecate it and later obsolete it in the release in which project loom enters main line,
>> I guess we (SAP) could agree with it.
>> It would be really helpful to communicate such plans before proposing deprecation.
>>
>> Best regards,
>> Martin
>>
>>
>>> -----Original Message-----
>>> From: Daniel D. Daugherty <daniel.daugherty at oracle.com>
>>> Sent: Freitag, 22. November 2019 16:06
>>> To: Andrew Dinn <adinn at redhat.com>; David Holmes
>>> <david.holmes at oracle.com>; Doerr, Martin <martin.doerr at sap.com>;
>>> 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
>>>
>>>> On 11/22/19 5:14 AM, Andrew Dinn wrote:
>>>> On 21/11/2019 21:31, David Holmes wrote:
>>>>> On 20/11/2019 2:51 am, Doerr, Martin wrote:
>>>>>> I think deprecating before publishing an evaluation or at least having
>>>>>> a discussion is not appropriate.
>>>>> Deprecation shows the intent that we (eventually) want to remove this
>>>>> and that people should try to avoid using it. If we don't actually
>>>>> deprecate it but just turn off then here is a likely scenario:
>>>> Who is this we?
>>>
>>> The way you edited the original thread makes it look like the "we" comes
>>> out of no where. Okay. Not sure why you did that, but here's the complete
>>> context:
>>>
>>> I posted this comment:
>>>
>>>> On 11/18/19 6:06 PM, Daniel D. Daugherty wrote:
>>>> 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.
>>>
>>> Martin posted this reply to my comment:
>>>> On 11/19/19 11:51 AM, Doerr, Martin wrote:
>>>>> 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.
>>>
>>> And David posted a reply to Martin's comment:
>>>
>>>> On 11/21/19 4:31 PM, David Holmes wrote:
>>>>> On 20/11/2019 2:51 am, Doerr, Martin wrote:
>>>>>> 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.
>>>>>
>>>>> Deprecation shows the intent that we (eventually) want to remove this
>>>>> and that people should try to avoid using it. If we don't actually
>>>>> deprecate it but just turn off then here is a likely scenario:
>>>>>
>>>>> - we turn of BL in 14
>>>>> - customer updates to 14 sees a performance issue, checks the release
>>>>> notes, sees BL is disabled and turns it back on.
>>>>> - customer continues on their merry way and feels no need to report
>>>>> back to OpenJDK that they need BL (even if we ask them to via release
>>>>> notes)
>>>>> - we get no feedback that BL is still useful and so we deprecate it
>>>>> in, say, 16
>>>>> - customer updates to 16 and gets the deprecation warning and then
>>>>> reports back that they need BL
>>>>>
>>>>> Alternatively we deprecate in 14 and customer lets us know straight
>>>>> away that it is still useful.
>>>
>>> So now David's use of "we" should be more clear. I do have to point out
>>> that my use of "we (Oracle)" was present in both Martin's reply and in
>>> David's reply to Martin, but for some reason you chose to edit it out.
>>> This makes your pushing back on David's use of an unqualified "we"
>>> questionable. Are you trying to be intentionally confrontational?
>>>
>>>
>>>> The premise of your scenario has built in the conclusion
>>>> that some of /us/ are questioning and thereby excluded our critique from
>>>> any chance of qualifying the proposed action.
>>>
>>> Hmmm... I don't see anyone excluding critiques here, but maybe I've missed
>>> something...
>>>
>>>
>>>> If the existence of such a consensus is not clear (and I suggest that
>>>> this thread makes that plain) and the evidence for arriving at such a
>>>> consensus is not compelling (ditto) and if the rest of the scenario will
>>>> likely play out as you suggest then that is a strong reason to
>>>> re-address the decision to switch the feature off, whether or not it is
>>>> deprecated at the same time.
>>>
>>> I suspect that "compelling" is in the eye of the beholder.
>>>
>>> Simply changing the default from true to false is pretty much a silent
>>> change in behavior even if we put out a release note. By deprecating at
>>> the same time, we'll have a visible diagnostic message if biased locking
>>> is enabled. That's much more likely to lead to feedback than a silent
>>> change in behavior.
>>>
>>>
>>>>> Alternatively we deprecate in 14 and customer lets us know straight away
>>>>> that it is still useful.
>>>> Alternatively, we come up with better evidence that it needs switching
>>>> off (and, possibly, deprecating).
>>>
>>> I wonder what would be considered acceptable "better evidence".
>>>
>>> Dan
>>>
>>>>
>>>> regards,
>>>>
>>>>
>>>> Andrew Dinn
>>>> -----------
>>>> Senior Principal Software Engineer
>>>> Red Hat UK Ltd
>>>> Registered in England and Wales under Company Registration No. 03798903
>>>> Directors: Michael Cunningham, Michael ("Mike") O'Neill
>>>>
>>>>
>>
> 