RFR: 8244684: G1 abuses StarTask to also include partial objarray scan tasks

[stefan.johansson at oracle.com]

Hi Kim,

On 2020-05-11 20:11, Kim Barrett wrote:
> Please review this cleanup of the parallel task queue usage by G1 non-full
> collections.  It currently uses queues with StarTask elements, except not
> really.  The oop* case is extended via an additional tag to also cover
> partial array scanning tasks.  This is accomplished by casting between oop
> and oop* and using misaligned oop* values.
> 
> A pair of new classes is added to address this.  PartialArrayScanTask wraps
> the source oop for a partial array scan.  ScannerTask is a discriminated
> union of oop*, narrowOop*, and PartialArrayScan, using a 2bit low tag in the
> relevant pointers.  This provides an easier to understand and safer API,
> without any performance cost.
> 
> This change also updates related nomenclature; there were a lot of places
> using "ref" / "reference" for these extended StarTasks, which isn't really
> appropriate for the partial array tasks.  So we now use "task" consistently.
> 
> Followup RFEs should look at making a similar change to ParallelGC, which
> does the same sort of thing.  And now that the support for this new task
> type is in a shared place (rather than two private variants in G1 and
> ParallelGC non-full collection classes) it might be worthwhile changing
> other places to use ScannerTask queues rather than separate queues for
> StarTasks and ObjArrayTasks.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8244684
> 
> Webrev:
> https://cr.openjdk.java.net/~kbarrett/8244684/open.00/
Really nice cleanup. Looks good in general, just a few small things:

gc/g1/g1ParScanThreadState.inline.hpp
---
  139 void 
G1ParScanThreadState::steal_and_trim_queue(ScannerTasksQueueSet 
*task_queues) {
  140   ScannerTask stolen_task;
  141   while (task_queues->steal(_worker_id, stolen_task)) {
  142     verify_task(stolen_task);
  143     dispatch_task(stolen_task);

First thing in dispatch_task() is verify_task() so this one on line 142 
could be removed.
---

gc/shared/taskqueue.hpp
---
  570 class PartialArrayScanTask {
  571   void* _p;
  ...
  578   oop to_source_array() const { return oop(_p); }

I see no reason not to use oop instead of void* here and the getter 
could be obj() as in the ObjArrayTask above. If you don't like obj() or 
want it to be more like the ScannerTask, maybe to_obj() or to_array() is 
better.
---

Kicked of a few fixed-ir jbb15 runs to verify that the pause times are 
not affected.

Thanks,
Stefan