RFR (S): 8185580: Refactor Threads::possibly_parallel_oops_do() to use Threads::parallel_java_threads_do()

[David Holmes]

On 17/10/2017 2:55 PM, Roman Kennke wrote:
> Am 17.10.2017 um 05:33 schrieb David Holmes:
>> Hi Roman,
>>
>> On 12/10/2017 5:30 AM, Roman Kennke wrote:
>>> This is a follow-up to my earlier safepoint parallel cleanup work.
>>>
>>> Currently we have 2 places where we apply the parallel claiming 
>>> protocol when iterating threads, that is 
>>> Threads::parallel_java_threads_do() and 
>>> Threads::possibly_parallel_oops_do(). In order to avoid code 
>>> duplication, the latter should call the former, using a private 
>>> ThreadClosure. We already had one bug (JDK-8185273) that was caused 
>>> by an inconsistency between the two.
>>>
>>> The only other user of parallel_java_threads_do() already filters out 
>>> any non-Java-threads, which means that doing the extra processing of 
>>> the VMThread should be ok. 
>>
>> "should" be okay? It's not at all clear to me what processing the 
>> VMThread where previously it was not processed, will actually do ??
> It ensures that both parallel thread iteration routines are doing the 
> same, and thus claim the same threads, which is later 
> assert_all_threads_are_claimed(). If we don't do this consistently, we 
> throw off the thread claiming mechanics.

Sorry that's not answering my question. Before we did not call

tc->do_thread(vmt);

for the VMThread, but now we do. So what is the affect of this change in 
behaviour?

AFAICS the "claiming" behaviour of both versions is the same.

Thanks,
David
-----

>>
>>> I renamed the method to parallel_threads_do() to reflect that it not 
>>> only does the Java threads.
>>
>> Given it takes an is_par argument it really should be 
>> possibly_parallel_threads_do()
> Ok. Differential diff:
> http://cr.openjdk.java.net/~rkennke/8185580/webrev.01.diff/ 
> <http://cr.openjdk.java.net/%7Erkennke/8185580/webrev.01.diff/>
> Full webrev:
> http://cr.openjdk.java.net/~rkennke/8185580/webrev.01/ 
> <http://cr.openjdk.java.net/%7Erkennke/8185580/webrev.01/>
> 
> Good?
> 
> Roman
>