RFR (S): 8245088: Always provide logs for G1 heap expansion calculations

[Thomas Schatzl]

Hi Kim, Stefan,

   thanks for your reviews.

On 18.05.20 16:23, Kim Barrett wrote:
>> On May 18, 2020, at 4:23 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>
>> Hi all,
[...]
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1HeapSizingPolicy.cpp
>    66 static void log(const char* prefix,
> 
> I think I'd prefer this be called log_expanion.

Done.

> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1HeapSizingPolicy.cpp
>    98     log("Heap expansion: Can not expand (heap already fully expanded)",
> 
> Is the "Can not expand..." phrase actually needed?  That information
> is also provided by the "fully expanded" part of the message, as well
> as the "resize by" being 0.
> 
> That phrase makes this case visually stand out more, but pushes
> interesting information way over to the right, possibly requiring
> horizontal scrolling to see, depending on one's window size.
> 
> That phrase also makes it just a little bit harder to use egrep to
> search for and parse these messages from a log file.
> 
> If that went away, the prefix would be identical for all callers, and
> could be dropped and hard-coded in the log_debug invocation.

Removed.

> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/g1/g1HeapSizingPolicy.cpp
>    76                             "threshold %1.2f%% pause time ratio %1.2f%% fully expanded %d "
> ...
>    83                             fully_expanded,
> 
> We have BOOL_TO_STR to print booleans as true/false rather than 1/0.
> 
> Alternatively, I think this isn't providing any additional information
> beyond what one gets from "resize by" being 0.
> 
> ------------------------------------------------------------------------------

Using BOOL_TO_STR now, an expansion of 0 bytes can also happen when 
there is no actual change in heap size (ie. while the internal state is 
updated).

Also the fix from Stefan has been incorporated into this change of course.

Webrevs:
http://cr.openjdk.java.net/~tschatzl/8245088/webrev.0_to_1/ (diff)
http://cr.openjdk.java.net/~tschatzl/8245088/webrev.1/ (full)

Thanks,
   Thomas