RFR 8240918: [REDO] Allow direct handshakes without VMThread intervention

[Patricio Chilano]

Hi Martin,

Thanks for looking at this.

On 4/2/20 10:15 AM, Robbin Ehn wrote:
> Hi Martin,
>
> On 2020-04-02 14:55, Doerr, Martin wrote:
>> Hi all,
>>
>> first of all, thanks, Patricio, for doing this.
>>
>> One question:
>> _pending_threads is used for synchronizing threads.
>> Don't we need release / acquire semantics?
>> I guess otherwise handshaker can read stale data from handshakee 
>> after observing is_completed.
>> Or did I miss anything?
>
> Reading stale should be fine in this case, since the counter goes from 
> n->0, thus you can only get false negatives from "bool is_completed()" ?
Right, as Robbin points out at most we will just have to loop again. 
Also, when the handshakee processes the handshake, right after updating 
_pending_threads, it will either signal the _processing_sem if there are 
no direct operations, or it will signal the _handshake_turn_sem, so that 
will provide the release already. The next read in the loop should read 
the updated value. Are you okay with that?

Thanks!

Patricio
> Thanks, Robbin
>
>>
>> Best regards,
>> Martin
>>
>>
>>> -----Original Message-----
>>> From: hotspot-runtime-dev <hotspot-runtime-dev-
>>> bounces at openjdk.java.net> On Behalf Of Robbin Ehn
>>> Sent: Donnerstag, 2. April 2020 13:36
>>> To: Patricio Chilano <patricio.chilano.mateo at oracle.com>; 
>>> hotspot-runtime-
>>> dev at openjdk.java.net
>>> Subject: Re: RFR 8240918: [REDO] Allow direct handshakes without
>>> VMThread intervention
>>>
>>> Hi Patricio,
>>>
>>> Thanks for fixing, looks good.
>>>
>>> /Robbin
>>>
>>> On 2020-04-01 19:45, Patricio Chilano wrote:
>>>> Hi all,
>>>>
>>>> Please review this redo for 8230594. Since the reason behind the 
>>>> Windows
>>>> timeouts turned out to be 8240902, this patch is almost identical 
>>>> to the
>>>> reverse of the backout modulo the rebase.
>>>>
>>>> There is only one small fix for an issue not identified in the 
>>>> original
>>>> patch which I explained in the comments. To avoid this possible 
>>>> issue of
>>>> deleting a JavaThread that called Thread::destroy_vm() while there are
>>>> handshakers blocked in the handshake_turn_sem semaphore, I added a
>>> check
>>>> to verify the JavaThread in question is not protected by some
>>>> ThreadsList reference before attempting the delete it. In case it is
>>>> protected, we need to at least clear the value for _thread_key to 
>>>> avoid
>>>> possible infinite loops at thread exit (this can happen in Solaris).
>>>>
>>>> The only other change is that I removed the check for local polls in
>>>> Handshake::execute_direct() since after the backout, thread-local
>>>> handshakes was implemented for arm32 which was the only remaining
>>> platform.
>>>>
>>>> I tested it several times in mach5 tiers 1-6 and once in t7 and saw no
>>>> failures.
>>>>
>>>> Bug:
>>>>      https://bugs.openjdk.java.net/browse/JDK-8240918
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~pchilanomate/8240918/v1/webrev/
>>>>
>>>>
>>>> Thanks,
>>>> Patricio