8387790: Shenandoah: Remove degenerated cycles by earthling-amzn · Pull Request #31797 · openjdk/jdk

earthling-amzn · 2026-07-06T19:58:24Z

When Shenandoah experiences a concurrent mode failure, it initiates a safepoint and attempts to finish the collection cycle there. This handoff between concurrent and degenerated cycles is error prone and has been the source of many, many defects. Removing degenerated cycles will simplify the code base, lower maintenance costs and allow the team to focus on more meaningful changes.

Threads which cannot allocate will stall. Threads which cannot evacuate will self-forward the object (this is not new behavior). Memory from regions with self-forwarded objects will be reclaimed simply by moving the region's top to the highest self-forwarded object in the region. More sophisticated reclamation from such regions will follow.

Additional behavior changes:

An allocation failure during old marking will run a global collection, rather than run young and risk starving old.
Allocation stalls are reported at the end of the process, reporting them for each cycle is deferred to another PR.
Allocation stalls are also used as a signal to increase the concurrent worker thread count (up to parallel thread count).
A Full GC will run after a thread cannot allocate after ShenandoahFullGCThreshold consecutive concurrent GCs.
The ShenandoahRegionCounter (used exclusively for the visualizer) reports cycles with evacuation failures instead of degenerated cycles.

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8387790: Shenandoah: Remove degenerated cycles (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/31797/head:pull/31797
$ git checkout pull/31797

Update a local copy of the PR:
$ git checkout pull/31797
$ git pull https://git.openjdk.org/jdk.git pull/31797/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 31797

View PR using the GUI difftool:
$ git pr show -t 31797

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/31797.diff

Using Webrev

Link to Webrev Comment

… from updating references Otherwise, assertions about objects being forwarded (even self-forwarded) may not hold.

Also, check for displaced mark words when making assertions about objects being self-forwarded.

…stroys lock bits)

…ions with failed evacuations

…unts

earthling-amzn · 2026-07-15T22:55:14Z


  ShenandoahHeapRegion* from_region = heap_region_containing(p);
  assert(!from_region->is_humongous(), "never evacuate humongous objects");
+  if (has_self_forwarded_objects() && from_region->has_self_forwards()) {


Currently, once a thread self-forwards an object, no other threads (not even a worker) will attempt further evacuations from the region. We could defer setting this flag until after a worker has tried to evacuate all it can, but as this region will only be partially recycled anyway, it is unclear if more aggressive evacuation would be worth it.

kdnilsen · 2026-07-17T22:31:27Z

Under "additional behavior changes":

Should we mention that a stall that happens while old marking is running concurrently will cause the next GC trigger to request a GLOBAL GC if the stalled thread has not yet been unblocked? (or clarify if I didn't get this quite right)
Can we elaborate on what is meant by "The region counter (visualizer) reports out cycles with evacuation failures as degenerated now"?

earthling-amzn · 2026-07-17T23:35:54Z

We have:

An allocation failure during old marking will run a global collection, rather than run young and risk starving old.

I will elaborate on 2.

kdnilsen

Thanks for this very thorough simplification of GenShen. I like the approach. I have a few questions embedded within my comments.

kdnilsen · 2026-07-17T22:42:19Z

  _cycle_start = os::elapsedTime();
+  cancel_trigger_request();
+  log_debug(gc, ergo)("Declined trigger count at start: %zu", _declined_trigger_count.load_relaxed());
 }


Note to self: In tip, we do not penalize if a concurrent GC "degenerates" through no fault of the triggering heuristic. We used a count of how many times we declined to trigger as an indication that the triggering heuristic might be responsible for degeneration. If it declined to trigger less than a small threshold value, then we say it was not at fault, and we do not apply a penalty.

I wonder if there's an equivalent notion now that we do not have degenerated cycles. I would expect that we apply a penalty if a GC cycle experiences allocation stalls and the scheduling heuristic had declined to trigger more than a specific threshold number of times.

I want to figure out if this mechanism is still present.

I see that ShenandoahHeuristics::record_concurrent_completion() still checks _declined_trigger_count before imposing a triggering penalty. I just need to find where its value is incremented.

ShenandoahHeuristics::decline_trigger() increments the count.

Yes, I tried to preserve all that machinery based on stalls.

kdnilsen · 2026-07-17T23:07:36Z

  _alloc_failure_full(0) {

-  Copy::zero_to_bytes(_degen_point_counts, sizeof(size_t) * ShenandoahGC::_DEGENERATED_LIMIT);
+  Copy::zero_to_bytes(_stall_counts, sizeof(size_t) * ShenandoahController::PHASE_LIMIT);


In the future, will we also report total time spent by mutator threads in stall in addition to the count of how many times threads experienced stalls?

kdnilsen · 2026-07-17T23:08:29Z

  _alloc_failure_full(0) {

-  Copy::zero_to_bytes(_degen_point_counts, sizeof(size_t) * ShenandoahGC::_DEGENERATED_LIMIT);
+  Copy::zero_to_bytes(_stall_counts, sizeof(size_t) * ShenandoahController::PHASE_LIMIT);


Probably would also like to see maximum time spent in any single stall on a per-thread basis.

kdnilsen · 2026-07-17T23:57:23Z

 }

 oop ShenandoahGenerationalHeap::evacuate_object(oop p, Thread* thread) {
  assert(thread == Thread::current(), "Expected thread parameter to be current thread.");


Note to self: The GC worker threads use an object iterator and closure to evacuate every object in a region. The closure calls this method for each marked object in the region. If a GC worker thread fails to evacuate an object within its "assigned" region, the closure will continue to invoke this method for subsequent marked objects within the region. The first failed allocation will cause the heap region to be identified as having self-forwarded objects, and this will cause the subsequent invocations of this method for objects residing in the same region to also be self forwarded.

kdnilsen · 2026-07-18T00:49:50Z


-  if (heap->is_degenerated_gc_in_progress()) {
+  if (heap->has_self_forwarded_objects()) {
    status |= (1 << 6);


Should we be using symbolic constants here?

Yes, but can we make that a TODO? It's somewhat orthogonal to this change.

kdnilsen · 2026-07-21T21:39:37Z

@@ -1151,7 +1145,14 @@ class ShenandoahEvacuationTask : public WorkerTask {
      assert(r->has_live(), "Region %zu should have been reclaimed early", r->index());
      _sh->marked_object_iterate(r, &cl);



This seems overly conservative. We don't know for sure that this thread introduced the self forwards. Maybe some other thread introduced the self-forwarded objects into this region.

I'm also concerned about overgeneralizing the cause for self-forwards. It is possible that the reason that OOM evac resulted in self-forwarded objects is because of a single very large object for which there is not a sufficiently large contiguous segment of memory to hold its copy, anywhere, even though there might be an abundance of memory available to hold the results of evacuating smaller objects. If that's the case, it is appropriate to self-forward that region, but it is not appropriate to shutdown this worker thread.

So there are two things I think we want to check here before shutting down this thread's efforts:

Confirm that I am the thread that caused this region to have self-forwarded objects, and

Confirm that the OOM during evac that I experienced is due to an inability to refresh a minimum-sized PLAB or GCLAB. If my existing GC/P LABs have at least min-size available memory and/or there is memory available to refresh my GC/P LABs, then I think we should not take this GC worker out of commission.

Willing to discuss/consider alternative perspectives, especially if we have data to contradict my recommendation here...

Yes, that's fair. In general, each region will be visited by only one worker, but it's possible that some rogue mutator with an exhausted PLAB could poison a region (or regions) before the worker gets to it. I shall think on this.

@kdnilsen , please see this commit specifically for this feedback: b117e36

kdnilsen · 2026-07-21T22:52:00Z

+  // Note: We may have spurious wakeups here when an old gc cycle completes.
+  // We don't have a mechanism to wait for specific types of cycles to complete.
+  // In general though, nobody should be notified when an old mark increment
+  // completes.


Note that the "final" old-mark increment may recycle immediate garbage. In some workloads, that is fairly common (e.g. diluvian). Would you want to special case that?

I will think on this. In this PR I combined 'alloc waiters' (waiting for memory) and 'gc waiters' (waiting for a complete cycle) because it seemed a simplification at that the time. Note that even before this change, both such waiters could be woken up spuriously for old mark increments (even when they don't reclaim memory). I have a follow up branch based on this PR that notifies waiters when final mark frees up garbage. I'll clean this up and make a dependent PR from this one.

@kdnilsen - Here is a dependent PR that is meant to address your feedback: #32034. It's not trivial so I didn't want to just fold it in here. The new PR also adds a feature to unblock alloc waiters for immediate garbage even when the cycle moves on to the evacuation phase. I co-opted this old ticket: https://bugs.openjdk.org/browse/JDK-8338534.

…ove-degenerated-cycles

openjdk · 2026-07-22T15:32:33Z

⚠️ @earthling-amzn This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

…cipate in safepoints

… in non-generational modes either

We don't want to reset this region's age and don't want to change its affiliation

earthling-amzn added 30 commits January 21, 2025 15:14

Prototyping

08fac07

Fix typos and other mistakes

0ae83f0

WIP: Leave regions with self-forwarded objects in the collection set

7b50c84

Merge from jdk/master

e5c267f

Merge remote-tracking branch 'jdk/master' into self-forwarding-lrb

a7ef4bc

WIP: Dealing with forwarded pointers everywhere

6924d64

WIP: Need to self forward all unevacuated objects in failed regions

d170b1b

WIP: Need to self forward all unevacuated objects in failed regions

4fc9c7f

WIP: Don't stomp on displaced mark word pointer

1fc7fbd

WIP: Stall allocation failure without cancelling

5c4bf0f

WIP: Only update self forwarded objects in cset regions

ea59dac

WIP: Clearing self forwarding bit needs to happen in a separate phase…

db2f6df

… from updating references Otherwise, assertions about objects being forwarded (even self-forwarded) may not hold.

Reset self forwarded pointers in a separate phase

195a73a

Also, check for displaced mark words when making assertions about objects being self-forwarded.

Also check for self-forwarded when mark word is not displaced

6458a06

Use object monitor table for safe self-forwarding

0ca9bd4

Do not encode pointer to self when setting self-forwarding bit (it de…

2bc6581

…stroys lock bits)

Do not update references in objects that have been evacuated from reg…

6788339

…ions with failed evacuations

Make gc workers follow same evacuation policy as mutators

036d5a5

Cannot use 'forwardee' if mark word is self-forwarded

3b06b37

WIP: self forwarding for generational mode

0527e1f

Merge remote-tracking branch 'jdk/master' into self-forwarding-lrb

e68f6c6

If it builds, we can debug it

3e25328

Merge remote-tracking branch 'jdk/master' into self-forwarding-lrb

065f072

Merge remote-tracking branch 'jdk/master' into self-forwarding-lrb

48f24ab

Remove IDE bonus includes

18fc52f

Checkpoint: Merge from upstream, it builds

f032506

checkpoint: TestAllocOutOfMemory passes for shen and genshen

854205e

remove unusable 'block' parameter

645fe4f

Do not jump to full gc when allocation fails (genshen)

5f01034

Just use total counts from CollectedHeap instead of dubious policy co…

0a3b7fa

…unts

earthling-amzn commented Jul 15, 2026

View reviewed changes

openjdk Bot mentioned this pull request Jul 16, 2026

8388368: Shenandoah: Remove mechanisms in Full GC to resume from degenerated cycle #31939

Open

4 tasks

earthling-amzn added 2 commits July 16, 2026 22:09

Don't need to look up the from_region

e3de290

Concurrent worker counts should adjust during a cycle

5d702e2

Make cancel_gc honor cause priorities as the control thread does

0af4ae0

kdnilsen reviewed Jul 21, 2026

View reviewed changes

earthling-amzn added 4 commits July 21, 2026 16:48

Add assertion, better comment

a2af48b

Test filling only old partially recycled regions

f0fdbd9

Fix comments

1e041dd

Merge branch 'test-fill-only-old-partially-recycled-regions' into rem…

ef3168f

…ove-degenerated-cycles

earthling-amzn added 2 commits July 22, 2026 15:34

Fix comments

13ac525

Strengthen requirement for GC workers to abandon further evacuations

b117e36

openjdk Bot added the merge-conflict Pull request has merge conflict with target branch label Jul 22, 2026

openjdk Bot mentioned this pull request Jul 23, 2026

8338534: Shenandoah: Handle alloc failure differently when immediate garbage is available #32034

Draft

4 tasks

Merge remote-tracking branch 'jdk/master' into remove-degenerated-cycles

56e854b

openjdk Bot removed the merge-conflict Pull request has merge conflict with target branch label Jul 24, 2026

earthling-amzn added 11 commits July 24, 2026 12:15

Fix wrong unit conversion

3077902

Do not lose pinned status for partially recycled regions

b756cbd

Self forwarding task should use a concurrent worker session and parti…

bcbcaaf

…cipate in safepoints

Simplify confusing comment

7bf2ed4

Disambiguate cause priorities for allocation failures

97e1cf3

Do not let high priority causes be preempted by lower priority causes…

3ba1e7c

… in non-generational modes either

Transition partially recycled region state in dedicated method

d8b60d7

We don't want to reset this region's age and don't want to change its affiliation

Remove vestigial un-self-forward mechanisms from degenerated cycle

7ab5ab4

Record alloc failure upgrade to full

afa8da1

Fix comments

3054e65

Merge remote-tracking branch 'jdk/master' into remove-degenerated-cycles

9ecb3db

		@@ -1151,7 +1145,14 @@ class ShenandoahEvacuationTask : public WorkerTask {
		assert(r->has_live(), "Region %zu should have been reclaimed early", r->index());
		_sh->marked_object_iterate(r, &cl);

Uh oh!

Conversation

earthling-amzn commented Jul 6, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

earthling-amzn Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kdnilsen commented Jul 17, 2026

Uh oh!

earthling-amzn commented Jul 17, 2026

Uh oh!

kdnilsen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openjdk Bot commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

earthling-amzn commented Jul 6, 2026 •

edited by openjdk Bot

Loading

earthling-amzn Jul 15, 2026 •

edited

Loading