Skip to content

8381550: Shenandoah: Fix confusing end of degenerated cycle log message #30684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pf0n wants to merge 1 commit into
base: master
Choose a base branch
from
+31 −5
Open

8381550: Shenandoah: Fix confusing end of degenerated cycle log message #30684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions src/hotspot/share/gc/shenandoah/shenandoahDegeneratedGC.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,24 @@ bool ShenandoahDegenGC::collect(GCCause::Cause cause) {
ShenandoahHeap* heap = ShenandoahHeap::heap();
if (heap->mode()->is_generational()) {
bool is_bootstrap_gc = heap->young_generation()->is_bootstrap_cycle();
heap->mmu_tracker()->record_degenerated(GCId::current(), is_bootstrap_gc);
const char* msg = is_bootstrap_gc? "At end of Degenerated Bootstrap Old GC": "At end of Degenerated Young GC";
const ShenandoahGenerationType generation_type = _generation->type();
heap->mmu_tracker()->record_degenerated(GCId::current(), is_bootstrap_gc, generation_type);
Copy link
Copy Markdown
Contributor

@earthling-amzn earthling-amzn Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we didn't need to duplicate this switch logic to compute what is essentially the same value, but I can't see a way around it that wouldn't turn into a huge refactor.

Copy link
Copy Markdown
Author

@pf0n pf0n Apr 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I initially didn't want the repetitive logic of the switch statements, but I noticed that the method record_degenerated calls ShenandoahMmuTracker::update_utilization which has the prefix At end of, while log_heap_status doesn't, so I can't pass in the same value for the record_degenerated method to avoid the repetition.

const char* msg = nullptr;
if (is_bootstrap_gc) {
msg = "At end of Degenerated Bootstrap Old GC";
} else {
switch (generation_type) {
case GLOBAL:
msg = "At end of Degenerated Global GC";
break;
case YOUNG:
msg = "At end of Degenerated Young GC";
break;
default:
ShouldNotReachHere();
break;
}
}
heap->log_heap_status(msg);
}
return true;
Expand Down
14 changes: 12 additions & 2 deletions src/hotspot/share/gc/shenandoah/shenandoahMmuTracker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,23 @@ void ShenandoahMmuTracker::record_mixed(size_t gcid) {
update_utilization(gcid, "Mixed Concurrent GC");
}

void ShenandoahMmuTracker::record_degenerated(size_t gcid, bool is_old_bootstrap) {
void ShenandoahMmuTracker::record_degenerated(size_t gcid, bool is_old_bootstrap, const ShenandoahGenerationType generation_type) {
if ((gcid == _most_recent_gcid) && _most_recent_is_full) {
// Do nothing. This is a redundant recording for the full gc that just completed.
} else if (is_old_bootstrap) {
update_utilization(gcid, "Degenerated Bootstrap Old GC");
} else {
update_utilization(gcid, "Degenerated Young GC");
switch (generation_type) {
case GLOBAL:
update_utilization(gcid, "Degenerated Global GC");
break;
case YOUNG:
update_utilization(gcid, "Degenerated Young GC");
break;
default:
ShouldNotReachHere();
break;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/shenandoah/shenandoahMmuTracker.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class ShenandoahMmuTracker {
void record_old_marking_increment(bool old_marking_done);
void record_mixed(size_t gcid);
void record_full(size_t gcid);
void record_degenerated(size_t gcid, bool is_old_boostrap);
void record_degenerated(size_t gcid, bool is_old_boostrap, const ShenandoahGenerationType generation_type);

// This is called by the periodic task timer. The interval is defined by
// GCPauseIntervalMillis and defaults to 5 seconds. This method computes
Expand Down