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

[Per Liden]

On 10/15/2018 07:05 PM, Erik Österlund wrote:
> 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/

Looks good.

/Per

> 
> 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
>>>>>>
>>>>>
>>>
>