RFR: 8256265 G1: Improve parallelism in regions that failed evacuation [v2]

[Thomas Schatzl]

On Fri, 3 Dec 2021 03:30:28 GMT, Hamlin Li <mli at openjdk.org> wrote:

> > * further this change implements limiting the number of threads for a particular subphase of a `G1BatchedTask`. That's new functionality, and while coincidentally we in the gc team talked about something like this (including the use of task queues) as a possibility for PR#6612 a few days, it would have been nice to discuss such a change - and as usual split it out before. :)
> 
> Not sure if I get the point, current implementation is calculate the thread sizing to set the terminator threads and task queue size at the end of G1PostEvacuateCollectionSetCleanupTask1 constructor. I think there is some better refactoring solution to get this done. I will check pr #6612 further to see what we can do first. :) (TO double check)
> 

I may have misread the code a bit; pr#6612 does not contain any similar changes - using the task queues has only been a thought.

> > I'm also not sure if it is useful to provide separate task queues for this mechanism, maybe the existing one can be reused.
> 
> Do you mean G1EvacFailureParScanTasksQueue* _task_queue in G1EvacFailureParScanState? it's just an OverflowTaskQueue, not a new structure. Would you mind to clarify further? :)

Yes, I meant reusing `G1EvacFailureParScanTasksQueue`, but that's not possible afaict - forget it.

> > At least not sure if 128k entries for each queue allocated at startup for every thread is a good idea (Did you measure how big they are? These tasks are significantly larger than others). Again, statistics can help.
> 
> Not quite get your point, would you mind to clarify futher about "128k entries for each queue allocated at startup for every thread"? :)

Just seeing, the task queues are dynamically allocated; still they are 128k entries * 32 bytes (i.e. 4MB / thread) in size. Do we need that big task queues (all the time)? And if not, how do we scale them? (The size is a template parameter right now....) Do we want to have changeable task queue sizes (only if using task queues is the way to go).
 
> 
> > I.e. this change needs something like [JDK-8140326](https://bugs.openjdk.java.net/browse/JDK-8140326) for example.
> 
> Yes, this is to be done.

Yes, a separate CR.
> 
> > But there may be other causes for the problem, and without providing statistics you can't tell.
> > Other requests:
> > 
> > * traceability: the change needs log messages at different levels, 
[...]
> 
> Can't agree more, I will first collect more statistics by adding logging, will do it in another PR first. (TODO new pr)

Things might be quicker if you sent an email about the proposed changes to hotspot-gc-dev first for discussion; otherwise every reviewer needs to check out, set everything up and run the code to see them...

Thanks,
  Thomas

-------------

PR: https://git.openjdk.java.net/jdk/pull/6627