RFR: 8377011: Shenandoah: assert_bounds should be only called when boundaries have changed [v2]

Kelvin Nilsen kdnilsen at openjdk.org
Mon Feb 9 21:23:37 UTC 2026


On Wed, 4 Feb 2026 00:30:18 GMT, Xiaolong Peng <xpeng at openjdk.org> wrote:

>> This fix the problem causing following 3 jtreg tests with tlab disabled to fail:
>> 
>> gc/shenandoah/TestRetainObjects.java#no-tlab 8361099 generic-all
>> gc/shenandoah/TestSieveObjects.java#no-tlab 8361099 generic-all
>> gc/shenandoah/TestSieveObjects.java#no-tlab-genshen 8361099 generic-all
>> 
>> 
>> 
>> The problem is assert_bounds is always called from function ShenandoahFreeSet::try_allocate_in, no matter the boundaries have changed or not after the allocation,  this is new behavior introduced by https://bugs.openjdk.org/browse/JDK-8365880, before which assert_bounds was only called after retiring a region.
>> 
>> The fix is to call assert_bounds only when boundaries have changed:
>> 1. When retire a region;
>> 2. When a the object is allocated in a new region - one region became non-empty.
>> 
>> There is also minor improvement to the if-else code in the PR.
>> 
>> Overall the change should be safe and only affect Shenandoah related tests, I have run Shenandoah test suite on MacOS.
>> 
>> Profiler data from the problematic jtreg test: [jtreg-gc_shenandoah_TestRetainObjects_no_tlab.html](https://github.com/user-attachments/files/25027324/jtreg-gc_shenandoah_TestRetainObjects_no_tlab.html) 
>> 
>> ### Tests
>> - [x] hotspot_gc_shenandoah
>> - [x] GHA
>
> Xiaolong Peng has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add assert_bounds_not_changed as Kelvin suggested

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 1197:

> 1195:          "Mutator humongous waste must match");
> 1196: }
> 1197: 

I realize I requested/suggested this, but upon further thought, I wonder if this is too strong.

IIRC, sometimes the leftmost_empty(partition) does not represent the left-most empty partition.  Sometimes it is just a left-most bound on the first empty partition.  The less demanding sanity check is probably:

assert((leftmost_empty(partition) == _max) || (leftmost_empty(partition >= leftmost(partition))

Similarly for rightmost_empty(partition).

As rewritten, I believe this is sufficient to handle the case that leftmost(partition) == _max or rightmost(partition) == -1.  Whenever leftmost(partition) == max or righmost(partition) == -1, the leftmost_empty(partition) should equal _max and righmost_empty(partition) should equal -1.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29537#discussion_r2776673512


More information about the hotspot-gc-dev mailing list