8368501: Shenandoah: GC progress evaluation does not use generation

Reviewed-by: ysr

src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp

@@ -115,8 +115,7 @@ void ShenandoahDegenGC::op_degenerated() {
   }
 #endif
 
-  ShenandoahMetricsSnapshot metrics;
-  metrics.snap_before();
+  ShenandoahMetricsSnapshot metrics(heap->free_set());
 
   switch (_degen_point) {
     // The cases below form the Duff's-like device: it describes the actual GC cycle,
@@ -308,10 +307,8 @@ void ShenandoahDegenGC::op_degenerated() {
     Universe::verify();
   }
 
-  metrics.snap_after();
-
   // Decide if this cycle made good progress, and, if not, should it upgrade to a full GC.
-  const bool progress = metrics.is_good_progress(_generation);
+  const bool progress = metrics.is_good_progress();
   ShenandoahCollectorPolicy* policy = heap->shenandoah_policy();
   policy->record_degenerated(_generation->is_young(), _abbreviated, progress);
   if (progress) {

src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp

@@ -104,21 +104,18 @@ void ShenandoahFullGC::entry_full(GCCause::Cause cause) {
 }
 
 void ShenandoahFullGC::op_full(GCCause::Cause cause) {
-  ShenandoahMetricsSnapshot metrics;
-  metrics.snap_before();
+  ShenandoahHeap* const heap = ShenandoahHeap::heap();
+
+  ShenandoahMetricsSnapshot metrics(heap->free_set());
 
   // Perform full GC
   do_it(cause);
 
-  ShenandoahHeap* const heap = ShenandoahHeap::heap();
-
   if (heap->mode()->is_generational()) {
     ShenandoahGenerationalFullGC::handle_completion(heap);
   }
 
-  metrics.snap_after();
-
-  if (metrics.is_good_progress(heap->global_generation())) {
+  if (metrics.is_good_progress()) {
     heap->notify_gc_progress();
   } else {
     // Nothing to do. Tell the allocation path that we have failed to make

src/hotspot/share/gc/shenandoah/shenandoahMetrics.cpp

@@ -28,69 +28,45 @@
 #include "gc/shenandoah/shenandoahHeapRegion.hpp"
 #include "gc/shenandoah/shenandoahMetrics.hpp"
 
-ShenandoahMetricsSnapshot::ShenandoahMetricsSnapshot() {
-  _heap = ShenandoahHeap::heap();
+ShenandoahMetricsSnapshot::ShenandoahMetricsSnapshot(ShenandoahFreeSet* free_set)
+  : _free_set(free_set)
+  , _used_before(free_set->used())
+  , _if_before(free_set->internal_fragmentation())
+  , _ef_before(free_set->external_fragmentation()) {
 }
 
-void ShenandoahMetricsSnapshot::snap_before() {
-  _used_before = _heap->used();
-  _if_before = _heap->free_set()->internal_fragmentation();
-  _ef_before = _heap->free_set()->external_fragmentation();
-}
-void ShenandoahMetricsSnapshot::snap_after() {
-  _used_after = _heap->used();
-  _if_after = _heap->free_set()->internal_fragmentation();
-  _ef_after = _heap->free_set()->external_fragmentation();
-}
-
-// For degenerated GC, generation is Young in generational mode, Global in non-generational mode.
-// For full GC, generation is always Global.
-//
-// Note that the size of the chosen collection set is proportional to the relevant generation's collection set.
-// Note also that the generation size may change following selection of the collection set, as a side effect
-// of evacuation.  Evacuation may promote objects, causing old to grow and young to shrink.  Or this may be a
-// mixed evacuation.  When old regions are evacuated, this typically allows young to expand.  In all of these
-// various scenarios, the purpose of asking is_good_progress() is to determine if there is enough memory available
-// within young generation to justify making an attempt to perform a concurrent collection.  For this reason, we'll
-// use the current size of the generation (which may not be different than when the collection set was chosen) to
-// assess how much free memory we require in order to consider the most recent GC to have had good progress.
-
-bool ShenandoahMetricsSnapshot::is_good_progress(ShenandoahGeneration* generation) {
+bool ShenandoahMetricsSnapshot::is_good_progress() const {
   // Under the critical threshold?
-  ShenandoahFreeSet* free_set = _heap->free_set();
-  size_t free_actual   = free_set->available();
+  const size_t free_actual = _free_set->available();
   assert(free_actual != ShenandoahFreeSet::FreeSetUnderConstruction, "Avoid this race");
 
-  // ShenandoahCriticalFreeThreshold is expressed as a percentage.  We multiple this percentage by 1/100th
-  // of the generation capacity to determine whether the available memory within the generation exceeds the
-  // critical threshold.
-  size_t free_expected = (ShenandoahHeap::heap()->soft_max_capacity() / 100) * ShenandoahCriticalFreeThreshold;
-
-  bool prog_free = free_actual >= free_expected;
-  log_info(gc, ergo)("%s progress for free space: %zu%s, need %zu%s",
-                     prog_free ? "Good" : "Bad",
-                     byte_size_in_proper_unit(free_actual),   proper_unit_for_byte_size(free_actual),
-                     byte_size_in_proper_unit(free_expected), proper_unit_for_byte_size(free_expected));
+  // ShenandoahCriticalFreeThreshold is expressed as a percentage.  We multiply this percentage by 1/100th
+  // of the soft max capacity to determine whether the available memory within the mutator partition of the
+  // freeset exceeds the critical threshold.
+  const size_t free_expected = (ShenandoahHeap::heap()->soft_max_capacity() / 100) * ShenandoahCriticalFreeThreshold;
+  const bool prog_free = free_actual >= free_expected;
+  log_info(gc, ergo)("%s progress for free space: " PROPERFMT ", need " PROPERFMT,
+                     prog_free ? "Good" : "Bad", PROPERFMTARGS(free_actual), PROPERFMTARGS(free_expected));
   if (!prog_free) {
     return false;
   }
 
   // Freed up enough?
-  size_t progress_actual   = (_used_before > _used_after) ? _used_before - _used_after : 0;
-  size_t progress_expected = ShenandoahHeapRegion::region_size_bytes();
-  bool prog_used = progress_actual >= progress_expected;
-  log_info(gc, ergo)("%s progress for used space: %zu%s, need %zu%s",
-                     prog_used ? "Good" : "Bad",
-                     byte_size_in_proper_unit(progress_actual),   proper_unit_for_byte_size(progress_actual),
-                     byte_size_in_proper_unit(progress_expected), proper_unit_for_byte_size(progress_expected));
+  const size_t used_after = _free_set->used();
+  const size_t progress_actual   = (_used_before > used_after) ? _used_before - used_after : 0;
+  const size_t progress_expected = ShenandoahHeapRegion::region_size_bytes();
+  const bool prog_used = progress_actual >= progress_expected;
+  log_info(gc, ergo)("%s progress for used space: " PROPERFMT ", need " PROPERFMT,
+                     prog_used ? "Good" : "Bad", PROPERFMTARGS(progress_actual), PROPERFMTARGS(progress_expected));
   if (prog_used) {
     return true;
   }
 
   // Internal fragmentation is down?
-  double if_actual = _if_before - _if_after;
-  double if_expected = 0.01; // 1% should be enough
-  bool prog_if = if_actual >= if_expected;
+  const double if_after = _free_set->internal_fragmentation();
+  const double if_actual = _if_before - if_after;
+  const double if_expected = 0.01; // 1% should be enough
+  const bool prog_if = if_actual >= if_expected;
   log_info(gc, ergo)("%s progress for internal fragmentation: %.1f%%, need %.1f%%",
                      prog_if ? "Good" : "Bad",
                      if_actual * 100, if_expected * 100);
@@ -99,9 +75,10 @@ bool ShenandoahMetricsSnapshot::is_good_progress(ShenandoahGeneration* generatio
   }
 
   // External fragmentation is down?
-  double ef_actual = _ef_before - _ef_after;
-  double ef_expected = 0.01; // 1% should be enough
-  bool prog_ef = ef_actual >= ef_expected;
+  const double ef_after = _free_set->external_fragmentation();
+  const double ef_actual = _ef_before - ef_after;
+  const double ef_expected = 0.01; // 1% should be enough
+  const bool prog_ef = ef_actual >= ef_expected;
   log_info(gc, ergo)("%s progress for external fragmentation: %.1f%%, need %.1f%%",
                      prog_ef ? "Good" : "Bad",
                      ef_actual * 100, ef_expected * 100);

src/hotspot/share/gc/shenandoah/shenandoahMetrics.hpp

@@ -25,22 +25,20 @@
 #ifndef SHARE_GC_SHENANDOAH_SHENANDOAHMETRICS_HPP
 #define SHARE_GC_SHENANDOAH_SHENANDOAHMETRICS_HPP
 
-#include "gc/shenandoah/shenandoahHeap.hpp"
+#include "gc/shenandoah/shenandoahFreeSet.hpp"
 
 class ShenandoahMetricsSnapshot : public StackObj {
 private:
-  ShenandoahHeap* _heap;
-  size_t _used_before, _used_after;
-  double _if_before, _if_after;
-  double _ef_before, _ef_after;
+  ShenandoahFreeSet* _free_set;
+  size_t _used_before;
+  double _if_before;
+  double _ef_before;
 
 public:
-  ShenandoahMetricsSnapshot();
+  explicit ShenandoahMetricsSnapshot(ShenandoahFreeSet* free_set);
 
-  void snap_before();
-  void snap_after();
-
-  bool is_good_progress(ShenandoahGeneration *generation);
+  // Decide if the GC made "good" progress (i.e., reduced fragmentation, freed up sufficient memory).
+  bool is_good_progress() const;
 };
 
 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHMETRICS_HPP