RFR: 8259668: Make SubTasksDone use-once [v2]

Thomas Schatzl tschatzl at openjdk.java.net
Fri Feb 5 09:37:43 UTC 2021


On Thu, 4 Feb 2021 15:41:59 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> After JDK-8260574, a instance of `SubTasksDone` is never reused, so part of its APIs could be revised: `clear()` and the code calling it is removed.
>> 
>> With this patch, `all_tasks_completed` contains only assertion. Kim suggested moving this assertion logic to `~SubTasksDone`, but that could defer the assertion violation. For example, in the case of `G1FullGCMarkTask::work`, there is a significant amount of code running btw the instance when all subtasks are claimed (where `all_tasks_completed` is called in this PR) and `~SubTasksDone`. In the interest of having more precise location where bugs may lie, I have kept `all_tasks_completed` in the original place. More comments on this are welcome.
>
> Albert Mingkun Yang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review

Looks good.

As an alternative to wrapping all (two) calls to `all_tasks_completel_impl` in `DEBUG_ONLY`, it is possible to declare it as `NOT_DEBUG_RETURN` outside of `#ifdef ASSERT` in the header file. The current approach is fine with me too.

An additional (as an extra CR, preexisting) cleanup could be moving more method definitions into the cpp file as they do not seem to be performance critical; but as mentioned, this is fine too.

Finally, could you sync the description of the CR in the JIRA with the PR one that there are no actual instances of reuse of this class any more? This is somewhat misleading as I was searching for actual reuse after reading the CR description (and skipping over the PR description) for a few minutes.

Thanks.

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

Marked as reviewed by tschatzl (Reviewer).

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


More information about the hotspot-dev mailing list