[9] RFR(S): 8059846: InstanceKlass should use MutexLockerEx to acquire OsrList_lock

[Dean Long]

On 10/14/2014 11:59 PM, Tobias Hartmann wrote:
> Hi Dean,
>
> On 15.10.2014 02:59, Dean Long wrote:
>> BTW, it looks like it does change the semantics slightly.  This code 
>> is now
>> executed inside the lock:
>>
>> 2881   if (best != NULL && best->comp_level() >= comp_level && 
>> match_level ==
>> false) {
>> 2882     return best;
>> 2883   }
>
> Yes, but that should be fine because the if statement only includes 
> fast and simple integer/pointer comparisons. I think defining an own 
> scope around the loop reduces code readability. What do you think?
>
I agree.

dl

> Best,
> Tobias
>
>>
>> dl
>>
>> On 10/14/2014 5:02 PM, David Holmes wrote:
>>> Tobias's change is fine but I get worried when I see:
>>>
>>> // This is a short non-blocking critical region, so the no safepoint 
>>> check is ok.
>>>
>>> as that is only one of the conditions needed to allow locking without
>>> safepoint checks!
>>>
>>> David H.
>>>
>>> On 15/10/2014 12:58 AM, Vladimir Kozlov wrote:
>>>> Looks good. Should be reviewed by runtime too since changes are in
>>>> instanceKlass.cpp
>>>>
>>>> Thanks,
>>>> Vlaidmir
>>>>
>>>> On 10/14/14 5:54 AM, David Chase wrote:
>>>>> Hello Tobias,
>>>>>
>>>>> Not a Reviewer here, but it looks correct to me.
>>>>> One question — above I see a NEEDS_CLEANUP notation.
>>>>> Do we know (does someone know?) if it refers to this change?
>>>>> If so, we could remove that :-).
>>>>>
>>>>> David
>>>>>
>>>>> On 2014-10-14, at 3:55 AM, Tobias Hartmann
>>>>> <tobias.hartmann at oracle.com> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> please review this small code cleanup that replaces the explicit
>>>>>> locking of OsrList_lock by a MutexLockerEx instantiation.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8059846
>>>>>> Webrev: http://cr.openjdk.java.net/~thartmann/8059846/webrev.00/
>>>>>>
>>>>>> Thanks,
>>>>>> Tobias
>>>>>
>>