RFR(M): 8017317: PPC64 (part 7): cppInterpreter: implement support for biased locking

[Lindenmaier, Goetz]

Hi David,

can I push this change now?  Or will the files be moved?
I prepared  a webrev without the shared change:
http://cr.openjdk.java.net/~goetz/webrevs/8017317-lock-2/

Best regards,
  Goetz.


-----Original Message-----
From: ppc-aix-port-dev-bounces at openjdk.java.net [mailto:ppc-aix-port-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
Sent: Freitag, 28. Juni 2013 00:07
To: David Holmes
Cc: ppc-aix-port-dev at openjdk.java.net; hotspot-dev at openjdk.java.net
Subject: Re: RFR(M): 8017317: PPC64 (part 7): cppInterpreter: implement support for biased locking

David,

Do you insist on moving cppInterpreter files into separate directory? If 
yes, we need to do that in our main sources to avoid merges nightmare. 
And I will do the move.

Thanks,
Vladimir

On 6/27/13 5:18 AM, David Holmes wrote:
> Vladimir,
>
> On 27/06/2013 9:34 AM, Vladimir Kozlov wrote:
>> Hi Goetz,
>>
>> On 6/26/13 7:30 AM, Lindenmaier, Goetz wrote:
>>> Hi,
>>>
>>> To use biased locking in our PPC64 VM,we fixed the biased locking
>>> implementation
>>> in the cppInterpreter.  We also added  BiasedLockingStatistic support.
>>
>> Oracle does not use cppInterpreter and no building and testing are done.
>> So we trust you to do testing of these changes.
>> I glanced on these changes and they look fine to me.
>>
>> One suggestion I heard from David H. is to move cppInterpreter related
>> files (all 6 of them) from interpreter/ directory to separate directory.
>> So we can see when changes does not affect our shared code.
>
> It is a general source of confusion to new hotspot engineers that there
> are actually two interpreters in one directory and that one is not used
> by the historical primary ports. It also isn't obvious that
> bytecodeInterpreter.cpp pertains to the cppInterpreter.
>
>>> We need an additional ordering operation in revoke_bias() when writing
>>> the mark.
>>
>> Why you need ordering only for locked objects in this code? And why here
>> at all? This code is executed at safepoint.
>
> There is one occurrence that is not executed at a safepoint - see
> BiasedLocking::Condition BiasedLocking::revoke_and_rebias. Though it
> seems to be operating only on current thread in that case.
>
> The use of release_set_mark seems consistent with other uses in
> synchronizer.cpp.
>
> David
> -----
>
>> thanks,
>> Vladimir
>>
>>> The change enables biased locking for ppc64 per default.
>>> http://cr.openjdk.java.net/~goetz/webrevs/8017317-lock/
>>>
>>> Please review and test this change.
>>>
>>> Best regards,
>>>    Goetz
>>>