RFR: 8287771: Remove useless G1 After GC summary refinement and sampling thread times [v2]

Thomas Schatzl tschatzl at openjdk.java.net
Tue Jun 14 09:05:00 UTC 2022


On Thu, 9 Jun 2022 14:43:54 GMT, tqxia <duke at openjdk.java.net> wrote:

>> In the "After GC summary" printed by gc+remset=trace and G1RemsetStatsSummaryPeriod > 1 we print refinement and sampling thread times. However during GC neither thread types are active, so it always prints zeros.
>> 
>> Remove the printing of these unnecessary values in the after gc summary.
>
> tqxia has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   8287771: Remove useless G1 After GC summary refinement and sampling thread times

Looks good apart from the parameter name.

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

> 2804: G1HeapPrinterMark::G1HeapPrinterMark(G1CollectedHeap* g1h) : _g1h(g1h), _heap_transition(g1h) {
> 2805:   // This summary needs to be printed before incrementing total collections.
> 2806:   _g1h->rem_set()->print_periodic_summary_info("Before GC RS summary", _g1h->total_collections(), true);

Suggestion:

  _g1h->rem_set()->print_periodic_summary_info("Before GC RS summary", _g1h->total_collections(), true /* include_vtimes */);


For bool parameters we often add the parameter name for clarity.

src/hotspot/share/gc/g1/g1RemSet.cpp line 1766:

> 1764: }
> 1765: 
> 1766: void G1RemSet::print_periodic_summary_info(const char* header, uint period_count, bool include_vtimes) {

Suggestion:

void G1RemSet::print_periodic_summary_info(const char* header, uint period_count, bool show_thread_times) {


I think the suggested parameter name is more clear than the original one.

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

Changes requested by tschatzl (Reviewer).

PR: https://git.openjdk.org/jdk/pull/9106



More information about the hotspot-gc-dev mailing list