Request for review (XS): 8006563: Remove unused ProfileVM_lock
David Holmes
david.holmes at oracle.com
Thu Jan 31 22:59:07 PST 2013
On 1/02/2013 4:54 PM, Rickard Bäckman wrote:
> That was the idea.
> However, can I have Ok for checking this into hs24 while waiting?
Sorry - ignore the hs25 comment - been looking at too many JDK review
requests.
Yes this seems fine for hs24.
David
> Thanks
> /R
>
> On Jan 21, 2013, at 11:33 PM, David Holmes wrote:
>
>> 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.
>>>>>
>
More information about the serviceability-dev
mailing list