RFR: 8289739: Add G1 specific GC breakpoints for testing
Thomas Schatzl
tschatzl at openjdk.org
Wed Jul 6 09:15:28 UTC 2022
On Wed, 6 Jul 2022 07:51:34 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
> It's unclear to me why "BEFORE REBUILD COMPLETED" is placed inside `phase_cleanup`; a more natural place to me is at the end of `G1ConcurrentMarkThread::phase_rebuild_remembered_sets`. (The same goes for the other pair of breakpoints.)
(Concurrent) rebuild completes before the cleanup pause. This follows the existing scheme, e.g. in `subphase_remark()` there is the `BEFORE MARKING COMPLETED` breakpoint before the remark pause (which completes the marking).
The Cleanup pause is also a kind of extension of the rebuild remset phase as it acts on actually this information (and it has not been "cleaning up" anything relevant for a long time since JDK 11 iirc).
This seems to be a separate, pre-existing issue.
>
> Sort of a preexisting issue but this change builds on top of it: "cleanup" can mean both the "cleanup pause" (`phase_cleanup`) or the "concurrent bitmap clearing" (`phase_clear_bitmap_for_next_mark`). IMO, it's better to get rid of such overloading.
The method names are different, one is called `cleanup` and the other `clear_bitmap_for_next_mark` phase for this reason. Maybe you suggest to rename the "Cleanup" pause and/or the "cleanup" use for the concurrent phase in the remaining code?
This renaming seems to be a separate (pre-existing) issue and related to above.
>
> (Not specific to this change. I also find the breakpoints naming pattern, `before-X-started & after-X-completed`, rather odd -- since they are break**points**, names reflecting a particular point in time, instead of a period, would have been more accurate, sth like `at-X-start & at-X-end`.)
The "at" seems to be more specific and could be changed separately and seems to be pre-existing.
-------------
PR: https://git.openjdk.org/jdk/pull/9376
More information about the hotspot-dev
mailing list