RFR: 8210064: ZGC: Introduce ZConcurrentRootsIterator for scanning a subset of strong IN_NATIVE roots concurrently

[Erik Österlund]

Hi,

This patch has been awaiting 3 patches to go in first due to unrelated 
test failures found in tier 6.
Here is a rebase of these fixes.

http://cr.openjdk.java.net/~eosterlund/8210064/webrev.03/

Compared to the last patch, I also

1) Introduce the STS joiner that is held while doing concurrent root 
iteration. The reason is that it is not safe to run this code through 
safepoints.
2) Treat the CLDs differently when marking vs heap iterating. Right now, 
I don't claim CLDs during heap iteration, because that will eventually 
break class unloading. Eventually, I will also need to walk different 
CLD sets during marking vs heap iteration (heap iteration will walk all 
CLDs, marking will only walk the strong ones), so the distinction is 
necessary.

Thanks,
/Erik

On 2018-09-04 12:23, Per Liden wrote:
> On 09/04/2018 12:17 PM, Erik Österlund wrote:
>> Hi Per,
>>
>> On 2018-09-04 11:47, Per Liden wrote:
>>> Hi Erik,
>>>
>>> On 09/04/2018 11:39 AM, Erik Österlund wrote:
>>>> Hi,
>>>>
>>>> Coleen pushed 8210155 before I pushed this. So now I need to wrap 
>>>> the CLDG iterations in a CLDG lock.
>>>> I made a patch that adds the lock and relaxes the locking assert 
>>>> appropriately so that the thread invoking the worker gang can take 
>>>> the lock. This implies that the worker threads will not own the 
>>>> lock, but that is okay.
>>>>
>>>> Incremental:
>>>> http://cr.openjdk.java.net/~eosterlund/8210064/webrev.01_02/
>>>>
>>>> Full:
>>>> http://cr.openjdk.java.net/~eosterlund/8210064/webrev.02/
>>>
>>> Looks good. Just one small thing. It seems the old 
>>> assert_locked_or_safepoint() can now be expressed as 
>>> assert_locked_or_safepoint_weak() plus some extra conditions.
>>
>> Thanks for the review. Unfortunately it looks like I can't express 
>> the strong one with the weak one as it has early exit code, and I 
>> need to change the weak one to return a boolean to make that work. Oh 
>> well.
>
> Ah, you're right of course. Ignore that.
>
> /Per
>
>>
>> Thanks,
>> /Erik
>>
>>> /Per
>>>
>>>>
>>>> Thanks,
>>>> /Erik
>>>>
>>>> On 2018-08-31 10:37, Erik Österlund wrote:
>>>>> Hi Per,
>>>>>
>>>>> On 2018-08-31 09:57, Per Liden wrote:
>>>>>> Hi Erik,
>>>>>>
>>>>>> On 08/30/2018 10:46 AM, Erik Österlund wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> We now have enough load barriers to support scanning of CLDs and 
>>>>>>> JNI handles concurrently. I propose to do that and move these 
>>>>>>> root sets out from ZRootsIterator, and hence the GC pause. They 
>>>>>>> will be scanned during concurrent marking (and heap iteration), 
>>>>>>> but no longer during relocation.
>>>>>>>
>>>>>>> I still perform ClassLoaderDataGraph::clear_claimed_marks() in 
>>>>>>> the pause because it seems cheap. But it can be moved out of the 
>>>>>>> pause when Coleen gets her new cool CLDG lock in.
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~eosterlund/8210064/webrev.00/
>>>>>>
>>>>>> I have some minor requests. Instead of listing them all, I 
>>>>>> attached a patch which addresses those.
>>>>>>
>>>>>> The main thing is that I don't think ZDriver should know about 
>>>>>> "concurrent roots", just that it's doing "mark" or "mark 
>>>>>> continue", so I suggest we turn that into a "bool initial" 
>>>>>> argument to mark() instead of exposing a mark_concurrent_roots() 
>>>>>> function.
>>>>>
>>>>> Sure, that makes sense.
>>>>>
>>>>> Thanks for the review!
>>>>>
>>>>> /Erik
>>>>>
>>>>>> The other things are minor style adjustments.
>>>>>> /Per
>>>>>>
>>>>>>>
>>>>>>> Bug:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8210064
>>>>>>>
>>>>>>> Tested through hs-tier1-3.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> /Erik
>>>>>
>>>>
>>