RFR: 8245022: ParallelGC abuses StarTask to also include partial objarray scan tasks

[Thomas Schatzl]

Hi,

On 20.05.20 14:49, Kim Barrett wrote:
>> On May 20, 2020, at 4:11 AM, stefan.johansson at oracle.com wrote:
>>
>> Hi Kim,
>>
>> On 2020-05-20 00:41, Kim Barrett wrote:
>>> Please review this change to ParallelGC to also use the recently added
>>> ScannerTask (JDK-8244684), eliminating some casts between oop and oop*
>>> and use of some misaligned oop* values.
>>> Also renamed G1's ScannerTasksQueue[Set] to G1ScannerTasksQueue[Set].
>>> I should have used those names in JDK-8244684.  This is consistent
>>> with the new PSScannerTasksQueue[Set] now being used by ParallelGC.
>>> The types used by G1 and Parallel are currently the same, but don't
>>> need to be.
>>> Also made a small simplification, reordering the template parameters
>>> for PSPromotionManager::copy_and_push_safe_barrier.  By making the
>>> promote_immediately flag first and the reference type second we can
>>> explicitly specify the non-deduced flag and allow the reference type
>>> to be deduced, rather than having to specify both explicitly in calls.
>> Nice!
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8245022
>>> Webrev:
>>> https://cr.openjdk.java.net/~kbarrett/8245022/open.00/
>> Looks good, one very small nit is the extra spaces left here:
>> share/gc/parallel/psPromotionManager.hpp
>> ---
>> 114  protected:
>> 115   static PSScannerTasksQueueSet* stack_array_depth()   { return _stack_array_depth; }
>>
>> They were pre-existing, but since we are touching that row, we can remove those spaces that seems to be left from some old alignment.
>> ---
>>
>> Thanks,
>> Stefan
> 
> Done.  I also took the opportunity to remove that unnecessary “protected:”, since
> PSPromotionManager is not used as a base class.
> 
> For the record, new webrevs:
> full: https://cr.openjdk.java.net/~kbarrett/8245022/open.01/
> incr: https://cr.openjdk.java.net/~kbarrett/8245022/open.01.inc/
> 
> The incremental change:
> 
> diff -r 72e792be5cf3 -r 2c299a64bed8 src/hotspot/share/gc/parallel/psPromotionManager.hpp
> --- a/src/hotspot/share/gc/parallel/psPromotionManager.hpp      Mon May 18 21:37:38 2020 -0400
> +++ b/src/hotspot/share/gc/parallel/psPromotionManager.hpp      Wed May 20 08:42:25 2020 -0400
> @@ -111,8 +111,8 @@
>                                       uint age, bool tenured,
>                                       const PSPromotionLAB* lab);
>   
> - protected:
> -  static PSScannerTasksQueueSet* stack_array_depth()   { return _stack_array_depth; }
> +  static PSScannerTasksQueueSet* stack_array_depth() { return _stack_array_depth; }
> +
>    public:
>     // Static
>     static void initialize();
> 

   still good. I doubt StefanJ has any problems with that change :)

Thanks,
   Thomas