8381550: Shenandoah: Fix confusing end of degenerated cycle log message #30684
8381550: Shenandoah: Fix confusing end of degenerated cycle log message #30684pf0n wants to merge 1 commit intoopenjdk:masterfrom
Conversation
|
👋 Welcome back pf0n! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
ysramakrishna
left a comment
There was a problem hiding this comment.
I think you may be able to refactor and common up some of the label generation (pun not intended) to reduce some of the repetition?
Looks good otherwise.
| 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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
Can we also fix this from It shouldn't be yelling about |
A degenerated global cycle in Generational Shenandoah will show up as a
Degenerated Young GCinstead of aDegenerated Global GCin the gc logs. Changed the hardcoded message by using the generation type of the gc to distinguish between a young and global gc.Before change:
After change:
Tested with GHA, local tier 1 tests, and created a test that produces a degenerated global cycle (did not push this per @earthling-amzn).
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30684/head:pull/30684$ git checkout pull/30684Update a local copy of the PR:
$ git checkout pull/30684$ git pull https://git.openjdk.org/jdk.git pull/30684/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 30684View PR using the GUI difftool:
$ git pr show -t 30684Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30684.diff
Using Webrev
Link to Webrev Comment