8266744: Make AbstractGangTask stack-allocatable only

[albertnetymk]

Remove the inheritance relation in class AbstractGangTask : public CHeapObj<mtInternal>, and export necessary classes.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8266744: Make AbstractGangTask stack-allocatable only

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/3918/head:pull/3918
$ git checkout pull/3918

Update a local copy of the PR:
$ git checkout pull/3918
$ git pull https://git.openjdk.java.net/jdk pull/3918/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 3918

View PR using the GUI difftool:
$ git pr show -t 3918

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/3918.diff

[bridgekeeper]

👋 Welcome back ayang! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@albertnetymk The following label will be automatically applied to this pull request:

• hotspot-gc

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (00f60ae5)

[kimbarrett]

I don't think this change should be made. HotSpot has many classes that are optionally c-heap allocated; there's nothing special about that concept. Forcing the explosure of RestorePreservedMarksTask just to avoid that doesn't seem like an improvement to me. Also, I really dislike the "Proxy" suffix renaming for the G1AbstractSubTask. (That's another consequence of exposing the RestorePreservedMarksTask class, though there are other, not necessarily better solutions, such as global namespace qualifiying references where needed.)

[albertnetymk]

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.

[bridgekeeper]

@albertnetymk This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@albertnetymk This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.