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

[Erik Österlund]

Hi Per,

Thanks for the review!

/Erik

On 2018-10-16 11:52, Per Liden wrote:
> 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
>>>>>>>
>>>>>>
>>>>
>>