RFR: 8212682: Avoid holding Compile_lock when blocking for GC in ObjArrayKlass::allocate_objArray_klass()
dean.long at oracle.com
dean.long at oracle.com
Mon Nov 19 18:26:26 UTC 2018
Yes, Reviewed!
dl
On 11/19/18 6:09 AM, Erik Österlund wrote:
> Hi Dean,
>
> May I interpret your commentary as Reviewed?
>
> Thanks,
> /Erik
>
> On 2018-10-24 22:20, dean.long at oracle.com wrote:
>> On 10/24/18 1:52 AM, Erik Österlund wrote:
>>> Hi Dean,
>>>
>>> On 2018-10-23 22:27, dean.long at oracle.com wrote:
>>>> Can allocate_objArray_klass() end up calling
>>>> SystemDictionary::add_to_hierarchy()? If so, then you'll end up
>>>> locking Compile_lock anway, but in the wrong order with respect to
>>>> MultiArray_lock.
>>>
>>> No, I can't see that allocate_objArray_klass() ever calls
>>> SystemDictionary::add_to_hierarchy(). If it did, we would assert
>>> that the Compile_lock is held; it never re-acquires the lock.
>>>
>>
>> OK, I forgot that mutex's can't be locked recursively.
>>
>> I found a clue to the reason for original 1998 change. This was also
>> added at the same time, to klassVtable::initialize_vtable():
>>
>> // We need the compile lock in order to force the super vtables and
>> // methodOop code pointers to stay constant.
>> // (Race scenario: compiler thread updates a vtable entry while
>> we're copying
>> // down entries from the superclass vtable.)
>> assert_lock(Compile_lock);
>>
>> By 1999, the vtable code changed a lot and the assert above went
>> away, so it was probably just an oversight that the extra uses of
>> Compile_lock weren't removed at the same time.
>>
>> dl
>>
>>> Thanks,
>>> /Erik
>>>
>>>> dl
>>>>
>>>> On 10/23/18 7:59 AM, Erik Österlund wrote:
>>>>> Hi,
>>>>>
>>>>> We occasionally blockingly wait for GC in allocations in
>>>>> ObjArrayKlass::allocate_objArray_klass(), while holding the
>>>>> Compile_lock.
>>>>> This is problematic for concurrent class unloading that needs to
>>>>> hold that lock in the unloading path. This introduces a potential
>>>>> deadlock situation.
>>>>>
>>>>> After staring enough at this code together with help from Coleen,
>>>>> I have convinced myself that the Compile_lock is in fact not
>>>>> needed in this path, and there is nothing to my knowledge it
>>>>> actually protects here. The vague comment next to the lock
>>>>> suggests its purpose is to protect vtable stubs. But it doesn't.
>>>>> There is a VtableStubs_lock for that, and I find no traces if the
>>>>> Compile_lock having anything to do with that.
>>>>>
>>>>> Coleen helped me trying to trace down where this locking code came
>>>>> from. It takes us back to before anyone was around, and does not
>>>>> seem to have changed in the past 2 decades, and its origins are a
>>>>> bit unclear. The theory is that perhaps vtable stubs used to be
>>>>> protected by the Compile_lock in ancient times, and the locking
>>>>> code is still around because nobody dared to touch it.
>>>>>
>>>>> Since both code analysis and mach5 suggest there is no reason for
>>>>> this lock to be held here, I propose to remove it with this patch.
>>>>>
>>>>> If anyone that has been around for more than 2 decades in HotSpot
>>>>> happens to know the reason why this locking was introduced, any
>>>>> commentary around that would be very appreciated.
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~eosterlund/8212682/webrev.00/
>>>>>
>>>>> Bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8212682
>>>>>
>>>>> Thanks,
>>>>> /Erik
>>>>
>>>
>>
>
More information about the hotspot-dev
mailing list