RFR: 8153745: Avoid spawning G1ParPreserveCMReferentsTask when there is no work to be done

[Stefan Johansson]

On 2016-04-11 19:49, Jon Masamitsu wrote:
>
>
> On 04/08/2016 04:45 AM, Thomas Schatzl wrote:
>> Hi,
>>
>> On Fri, 2016-04-08 at 13:10 +0200, Stefan Johansson wrote:
>>> Hi,
>>>
>>> Please review this enhancement to avoid spawning tasks when there is
>>> no
>>> work to do:
>>> https://bugs.openjdk.java.net/browse/JDK-8153745
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sjohanss/8153745/hotspot.00/
>>>
>>> Summary:
>>> During post_evacuate_collection_set the call to preserve_cm_referents
>>> ensures that no referents discovered by the concurrent reference
>>> processor are lost. This is done unconditionally and sometimes a lot
>>> of
>>> tasks are spawned just to find that there is no work to be done. This
>>> enhancement avoids the unnecessary work by first checking that the
>>> cm-ref-processor has at least some discovered reference to take care
>>> of.
>>    - I would prefer if the referenceprocessor would track whether there
>> are any references in it by itself by setting a flag while adding a
>> reference. I would think that this would be noise in gathering
>> references anyway.
>> While I do not think it is very problematic, consider machines having
>> thousands of threads (which is reality right now).
>>
>> Otherwise at least, if there is no way that any references could have
>> been enqueued because concurrent marking is not running, add an early
>> exit.
>
> Thomas,
>
> I don't understand the need to look for the case where concurrent marking
> is not running. "No reference to process" is the real condition we 
> need to
> skip the processing so why the check whether concurrent marking is
> running/not running?  Also, I don't trust flags about whether a phase
> running or not.  While they may be right today, code gets moved around
> and bugs happen.
>
Hi Jon,

The ref_processor_cm() can only have discovered references during a 
concurrent cycle otherwise discovery is disabled and this reference 
processor is not used. I agree that it is somewhat dangerous to trust 
these state flags, especially since there are more than one trying to 
describe the same thing. I still think this is a good middle road to 
avoid scanning too many lists on big systems when we know there will be 
no discovered references.

Thanks,
Stefan
> Jon
>
>>
>>    - the code should not add timing information in case there has been
>> no work. It will automatically show "skipped" in that case. I.e. please
>> move the timing stuff inside the if-block.
>>
>> There is a difference between taking "0.0" time and not having executed
>> that is sometimes interesting to know.
>>
>> Thanks,
>>    Thomas
>>
>