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

[Per Liden]

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