RFR: 8361099: Shenandoah: Improve heap lock contention by using CAS for memory allocation [v5]

[Kelvin Nilsen]

On Wed, 5 Nov 2025 19:00:03 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> Xiaolong Peng has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 135 commits:
>> 
>>  - Merge branch 'openjdk:master' into cas-alloc-1
>>  - Merge branch 'openjdk:master' into cas-alloc-1
>>  - format
>>  - Merge branch 'openjdk:master' into cas-alloc-1
>>  - Merge branch 'openjdk:master' into cas-alloc-1
>>  - Merge branch 'master' into cas-alloc-1
>>  - Move ShenandoahHeapRegionIterationClosure to shenandoahFreeSet.hpp
>>  - Merge branch 'openjdk:master' into cas-alloc-1
>>  - Fix errors caused by renaming ofAtomic to AtomicAccess
>>  - Merge branch 'openjdk:master' into cas-alloc-1
>>  - ... and 125 more: https://git.openjdk.org/jdk/compare/2f613911...e6bfef05
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 848:
> 
>> 846: HeapWord* ShenandoahFreeSet::allocate_with_affiliation(Iter& iterator, ShenandoahAffiliation affiliation, ShenandoahAllocRequest& req, bool& in_new_region) {
>> 847:   for (idx_t idx = iterator.current(); iterator.has_next(); idx = iterator.next()) {
>> 848:     ShenandoahHeapRegion* r = _heap->get_region(idx);
> 
> I wonder if we could refine this a little bit.  When the region is moved into the "directly allocatable" set, wouldn't we remove it from its partition?  Then, we wouldn't have to test for !r->reserved_for_direct_allocation() here because the iterator wouldn't produce it. 
> 
> We could maybe replace this test with an assert that !r->reserved_for_direct_allocation().

Same issue in other uses of the allocation iterator.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 2280:
> 
>> 2278: }
>> 2279: 
>> 2280: class DirectAllocatableRegionRefillClosure final : public ShenandoahHeapRegionIterationClosure {
> 
> I don't think we want to subclass ShenandoahHeapRegionIterationClosure here.  That iterates over all 2000 regions.  We only want to iterate over the 13 Directly allocatable regions.  Maybe we don't even need/want a closure iterator here.  We could just write a loop.

I think we should be borrowing from this code when replenishing the regions that are ready to be retired:

  if (_partitions.alloc_from_left_bias(ShenandoahFreeSetPartitionId::Mutator)) {
    // Allocate from low to high memory.  This keeps the range of fully empty regions more tightly packed.
    // Note that the most recently allocated regions tend not to be evacuated in a given GC cycle.  So this
    // tends to accumulate "fragmented" uncollected regions in high memory.
    ShenandoahLeftRightIterator iterator(&_partitions, ShenandoahFreeSetPartitionId::Mutator);
    return allocate_from_regions(iterator, req, in_new_region);
  }

  // Allocate from high to low memory. This preserves low memory for humongous allocations.
  ShenandoahRightLeftIterator iterator(&_partitions, ShenandoahFreeSetPartitionId::Mutator);
  return allocate_from_regions(iterator, req, in_new_region);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26171#discussion_r2495761580
PR Review Comment: https://git.openjdk.org/jdk/pull/26171#discussion_r2496372013