Request for review (XS): 8006563: Remove unused ProfileVM_lock

[David Holmes]

On 22/01/2013 12:09 AM, Rickard Bäckman wrote:
> Yes, that code has changed. Checked in to hs24.

Okay but this is a review for hs25 ;-) So I assume that change will be 
there "real soon now". :)

David

> /R
>
> 21 jan 2013 kl. 02:59 skrev David Holmes<david.holmes at oracle.com>:
>
>> On 18/01/2013 11:45 PM, Rickard Bäckman wrote:
>>> Aleksey,
>>>
>>> thanks for your review!
>>>
>>> a) It was before on of my own changes used in os_solaris.cpp (I think, for synchronization support for Suspend/Resume).
>>> I don't think we wanted something external to mess with that lock.
>>
>> Seems to be used here:
>>
>> ./os/solaris/vm/os_solaris.cpp:
>>
>> 4265   GetThreadPC_Callback  cb(ProfileVM_lock);
>>
>> Is this code already undergoing removal as part of the JFR changes?
>>
>> Thanks,
>> David
>> -----
>>
>>
>>> b) I've changed the indentation slightly.
>>> Updated webrev at http://cr.openjdk.java.net/~rbackman/8006563.2/ (or at least currently copying…)
>>>
>>> /R
>>>
>>> On Jan 18, 2013, at 2:12 PM, Aleksey Shipilev wrote:
>>>
>>>> On 01/18/2013 04:58 PM, Rickard Bäckman wrote:
>>>>> http://cr.openjdk.java.net/~rbackman/8006563/
>>>>
>>>> Looks good to me (not a Reviewer), modulo:
>>>> a) Are we sure this thing is not acquired in some weird way, i.e.
>>>> through JVMTI, SA, or whatnot?
>>>> b) The formatting of the predicate does not follow it's structure, I
>>>> think it should be:
>>>>      ...
>>>>      this != Interrupt_lock&&
>>>>      !(this == Safepoint_lock&&
>>>>        contains(locks, Terminator_lock)&&
>>>>        SafepointSynchronize::is_synchronizing())) {
>>>>
>>>> This way it is more obvious SafepointSynchronize::is_synchronizing()) is
>>>> the !(...) group.
>>>>
>>>> -Aleksey.
>>>