RFR: 8308500: ZStatSubPhase::register_start should not call register_gc_phase_start if ZAbort::should_abort()

[Axel Boldt-Christmas]

On Mon, 22 May 2023 10:52:44 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> `ZStatSubPhase::register_start` should not call `register_gc_phase_start` if `ZAbort::should_abort()` is true. This will cause an unbalanced push and pop behaviour of the phase stack as `ZStatSubPhase::register_end` stops popping (and sending events) after the aborting has started. This will create an issue if more subsequent sub-phases are added in-between two abort points as the phase stack may overflow. 
>> 
>> Simply avoid pushing new phases when aborting has started solves this issue.
>
> src/hotspot/share/gc/z/zStat.cpp line 811:
> 
>> 809: 
>> 810: void ZStatSubPhase::register_start(ConcurrentGCTimer* timer, const Ticks& start) const {
>> 811:   if (timer != nullptr && !ZAbort::should_abort()) {
> 
> `register_end` also skips the logging part if we should abort. Should we do the same for the `register_start`?

My initial implementation had that. (just an early return)
But thought it might be interesting to see if there ever is an issue with the abort code to have the phase starts in the log. The end logging is less interesting as we are aborting. This is how single generation also does it.
Given that we only log sub phases on debug, maybe this is less useful as we will miss it in the normal gc* logs.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14075#discussion_r1200359600