RFR: 8266744: Make AbstractGangTask stack-allocatable only

Albert Mingkun Yang ayang at openjdk.java.net
Mon May 10 20:35:56 UTC 2021


On Mon, 10 May 2021 11:54:28 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> HotSpot has many classes that are optionally c-heap allocated; there's nothing special about that concept.

`AbstractGangTask` has a few dozens of subclasses, all of them are stack-allocated, except the recently introduced case of `RestorePreservedMarksTask`. Using stack-allocation is not only easier to read/reason, but also avoids potentially expensive heap allocation.

> Forcing the explosure of RestorePreservedMarksTask just to avoid that doesn't seem like an improvement to me.

What are the downsides of making it public? OTOH, having all subclassess of `AbstractGangTask` "newable" encourages unstructured usage of it, which has a much larger impact, as it has many subclasses.

> Also, I really dislike the "Proxy" suffix renaming for the G1AbstractSubTask.

My original reasoning is that `RestorePreservedMarksTaskProxy` does nothing but delegate the work, `_task.work(worker_id);`, so it's not really a task per se. However, after closer inspection at some surrounding code, I do agree there's sth weird with such suffix.

1. `class ...Proxy : ...Task` does seem a bit off.
2. In the constructor of `G1PostEvacuateCollectionSetCleanupTask2`, 


add_..._task(new ...Task);
add_..._task(new ...Task);
add_..._task(new ...TaskProxy); // does stand out
add_..._task(new ...Task);


Maybe `RestoreMarksTask`? I wanna avoid name collision, not only from C++ perspective, but from readability aspect as well.

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

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



More information about the hotspot-gc-dev mailing list