RFR (jdk10): JDK-8180175: ObjectSynchronizer only needs to iterate in-use monitors

[David Holmes]

On 18/05/2017 7:00 PM, Robbin Ehn wrote:
> Hi,
>
> On 05/18/2017 10:10 AM, David Holmes wrote:
>> On 18/05/2017 6:08 PM, Roman Kennke wrote:
>>> Am 18.05.2017 um 10:03 schrieb David Holmes:
>>>> On 18/05/2017 5:54 PM, Roman Kennke wrote:
>>>>> Am 18.05.2017 um 04:07 schrieb David Holmes:
>>>>>> Roman,
>>>>>>
>>>>>> I just pushed this but now I think there is a problem with this
>>>>>> optimization.
>>>>>>
>>>>>> When a thread terminates, omFlush is used to transfer the thread's
>>>>>> inUseList to the global inUseList. This happens after the thread has
>>>>>> been removed from the _threads_list and can run concurrently with a
>>>>>> safepoint. This means that there is a window of time where the
>>>>>> monitors in the thread's inUseList will not be seen by the safepoint
>>>>>> oops_do processing:
>>>>>> - the thread oops_do won't be called because the thread is not in the
>>>>>> thread's list
>>>>>> - the monitors are not yet in the global in-use-list
>>>>>>
>>>>>> Did I overlook something?
>
> Good catch!
>
>>>>>
>>>>> Ugh. How ugly :-)
>>>>>
>>>>> Is there a reason why omFlush cannot be called from the same block
>>>>> where
>>>>> the thread is removed from _thread_list, under the Threads_lock ?
>
> Couldn't omFlush be done before Threads_lock in Thread::remove, while
> the thread is still on threads list?
> I do not see the reason why you want to hold the Threads_lock? To me it
> looks like gListLock is enough?

Not sure it makes any practical difference. If you do it before then the 
thread is still part of safepoint protocol, so no safepoint can occur 
while calling omFlush. If you do it while holding Threads_lock then no 
safepoint can occur while doing omFlush.

David

> /Robbin
>
>>>>
>>>> I think the explicit intent is to let it run independent of safepoints
>>>> (which holding the Threads_lock would affect).
>>>
>>> But Thread::remove() *is* going to block on Threads_lock. If I read the
>>> code correctly, omFlush() is called right after we give up Threads_lock
>>
>> Right - in the spirit of "critical sections should be as short as
>> possible" the omFlush is done outside of the holding of the threads_lock.
>>
>>> (thread_main_inner()). Stuff under gListLock in omFlush() is only doing
>>> some simple re-linking of the linked lists. I'll give that a try with
>>> some very careful checking ...
>>
>> It's the other uses of the gListlock that need to be examined.
>>
>> David
>>
>>> Roman
>>>