RFR: 8269022: Put evacuation failure string directly into gc=info log message

[Leo Korinth]

On Mon, 21 Jun 2021 13:18:11 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

> Hi all,
> 
>   can I have reviews for this change that puts the evacuation failure marker from a separate log line into the GC timing message. I.e. instead of
> 
> 
> Pause Young (Normal) (G1 Evacuation Pause)
> To-space exhausted
> 
> into
> 
> 
> Pause Young (Normal) (Evacuation Failure) (G1 Evacuation Pause) 
> 
> 
> This is imho easier to read, better to process and less wasteful with log space.
> 
> Testing: tier1, gc/g1 tests

I have some comments, but otherwise it looks good to me.

src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 2917:

> 2915:   GCTraceTime(Info, gc) _tt;
> 2916: 
> 2917:   char* update_young_gc_name() {

this can be made a const char*, right?

src/hotspot/share/gc/g1/g1CollectedHeap.cpp line 2938:

> 2936:   ~G1YoungGCTraceTime() {
> 2937:     update_young_gc_name();
> 2938:   }

Do I understand correctly that we update the name in _young_gc_name_data in the destructor, and then depend on that _tt does not  copy that value earlier --- only the pointer? This seems quite fragile if GCTraceTimeWrapper would change, and I think would at least warrant a comment in the constructor that the string *will* be updated in the destructor.

test/hotspot/jtreg/gc/g1/TestEvacuationFailure.java line 70:

> 68: 
> 69:         public static void main(String [] args) {
> 70:             largeObject = new byte[16*1024*1024];

space around asterisks

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

Marked as reviewed by lkorinth (Committer).

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