On Fri, 3 Nov 2023 12:44:10 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:
>> Thomas Schatzl has updated the pull request incrementally with one >> additional commit since the last revision: >> >> ayang review - renamings + documentation > > src/hotspot/share/gc/g1/g1YoungCollector.cpp line 87: > >> 85: GCCause::to_string(_pause_cause), >> 86: _collector->evacuation_pinned() ? " (Pinned)" : "", >> 87: _collector->evacuation_alloc_failed() ? " (Allocation >> Failure)" : ""); > >> GC(6) Pause Young (Normal) (Pinned) (Evacuation Failure) > > I wonder if the last two can be merged into one `()`, sth like `(Pinned / > ...)`, because they are on the same abstraction level. Parsing the separate components is easier :) Not sure if these tags in any way ever indicated some level of abstraction. I do not have a strong opinion here. The combinations (Pinned) (Allocation Failure) (Pinned + Allocation Failure) // or the other way around, or some other symbol for "+" or no symbol at all? are fine with me (and I thought about doing something more elaborate here), but my concern has been that any complicated string makes it less unique (e.g. `(Allocation Failure)` vs. "Allocation Failure") and adds code both to implement and parse the result. Much more disrupting is likely that there is no "Evacuation Failure" string any more. But log messages are not part of the external interface, and we should not want to change them just because. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16342#discussion_r1381716129
