RFR: 8217327: G1 Post-Cleanup region liveness printing should not print out-of-date efficiency [v3]

[Joakim Nordström]

On Thu, 28 Jan 2021 09:14:37 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Joakim Nordström has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Added format buffer and fixed duplicated code
>
> src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 2983:
> 
>> 2981: 
>> 2982:   if(gc_eff < 0) {
>> 2983:     snprintf(gc_efficiency, G1PPRL_DOUBLE_FORMAT_LEN+1, G1PPRL_DOUBLE_H_FORMAT, "-");
> 
> snprintf is fine with me too, but I had imagined something like this:
> 
> FormatBuffer<> efficiency(""); //  maybe better name this
> if (gc_eff < 0.0) {
>   efficiency.append("-");
> } else {
>   efficiency.append(G1PPRL_DOUBLE_H_FORMAT, gc_eff);
> }
> 
> and in the `log_trace` use `%s` and `efficiency.buffer()`. That seems a lot easier to understand (for me) than wrangling with `snprintf`.
> 
> Maybe there is a reason to not use this?
> 
> Also, I am not sure that using `G1PPRL_DOUBLE_H_FORMAT` for this `snprintf` here to print only `-` is really correct. I would have naively have expected to require `%s`.

Again, you're absolutely right. 
I looked for a FormatBuffer class in the codebase when you mentioned it (kind of what I wanted from the beginning). For whatever reason I couldn't find one (typo, temporary blindness, ignorance??). So I went with snprintf (even though I don't particular like it). 
I'll fix this. Sorry for all the bother.

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

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