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

[Jon Masamitsu]

On 4/12/2016 5:02 AM, Stefan Johansson wrote:
>
>
> 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.

Ok.  Changes look good.

Jon

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