RFR: 8221102: Allow GC threads to participate in threads claiming protocol

[Roman Kennke]

Am 25.03.19 um 20:42 schrieb Kim Barrett:
>> On Mar 22, 2019, at 6:00 PM, Roman Kennke <rkennke at redhat.com> wrote:
>>
>>>> Please review this change to the thread claiming protocol for parallel
>>>> iteration over threads. Previously, the protocol used a 1bit claim
>>>> counter plus zero as a "never claimed" marker. The new protocol uses a
>>>> uintx counter, while retaining zero as a "never claimed" marker.
>>>>
>>>> […]
>>>> CR:
>>>> https://bugs.openjdk.java.net/browse/JDK-8221102
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~kbarrett/8221102/open.00/
>>>>
>>>> Testing:
>>>> mach5 tier1-5.
>>>
>>> This would work.
>>>
>>> I am going back and forth with this in my head. I believe the simpler,
>>> more usable and perhaps less surprising approach might be to simply
>>> always iterate all threads, and do not even let other code use the
>>> claiming protocol, and care about its idiosyncrasies:
>>>
>>> […]
>>> I don't have a strong preference for your vs my solution, except that I
>>> want this bug fixed. :-) Just wanted to throw it out so that others can
>>> chime into the discussion.
>>
>> And in-fact, both solutions could be combined, they are not mutually
>> exclusive. Not sure what the point would be though ;-) I think it's nice
>> to not expose the claiming logic to any outside code though.
>>
>> Roman
> 
> It *might* work.  And it was one of the options I initially suggested.
> But all of the relevant closures need to be examined to ensure they
> don't have any assumptions that would make them not work properly when
> applied to some non-VM NJT.  I don't want to do that audit as part of
> fixing the bug if that can be avoided.  And I think extending the
> counter as I've proposed avoids the need for that audit.  It also has
> the benefit of allowing subset iterations, which seems like it could
> be useful in other ways (though we presently only have the two sets).
> 
> We could give less exposure to the claiming mechanism by providing a
> second function similar to the existing possibly_parallel_threads_do
> that does hit all the threads.  The only problem I have with that is
> naming.  Maybe the old function should be renamed to
> possibly_parallel_java_threads_and_vm_thread_do, for consistency with
> the serial function.

Zhengyu found a problem with my approach to include all workers in the
protocol: workers that would be created at safepoints would start out
with 0 claim token, and the final verification would fail because it
wants all threads to be at global thread_parity. It could be fixed
fairly easily by letting all threads start with current global parity.
But as you say, this might just be one case of assumptions and there
might be more lurking around. So let's go with your approach for now.

Roman