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

[Xiaolong Peng]

On Tue, 6 Jan 2026 23:11:19 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 265 commits:
>> 
>>  - Merge branch 'openjdk:master' into cas-alloc-1
>>  - Fix build error after merging from tip
>>  - Merge branch 'master' into cas-alloc-1
>>  - Merge branch 'master' into cas-alloc-1
>>  - Some comments updates as suggested in PR review
>>  - Fix build failure after merge
>>  - Expend promoted from ShenandoahOldCollectorAllocator
>>  - Merge branch 'master' into cas-alloc-1
>>  - Address PR comments
>>  - Merge branch 'openjdk:master' into cas-alloc-1
>>  - ... and 255 more: https://git.openjdk.org/jdk/compare/de81d389...cf13b7b5
>
> src/hotspot/share/gc/shenandoah/shenandoahAllocator.cpp line 280:
> 
>> 278:     // Step 2: allocate region from FreeSets to fill the alloc regions or satisfy the alloc request.
>> 279:     ShenandoahHeapRegion* reserved[MAX_ALLOC_REGION_COUNT];
>> 280:     int reserved_regions = _free_set->reserve_alloc_regions(ALLOC_PARTITION, refreshable_alloc_regions,
> 
> I request we get rid of the min_free_words argument to free_set->reserve_alloc_regions().  See comments in the called function.

I have replied in previous comments, we keep it but passing  PLAB::min_size() value for now.

> src/hotspot/share/gc/shenandoah/shenandoahAllocator.cpp line 304:
> 
>> 302:         log_debug(gc, alloc)("%sAllocator: Storing heap region %li to alloc region %i",
>> 303:           _alloc_partition_name, reserved[i]->index(), refreshable[i]->alloc_region_index);
>> 304:         AtomicAccess::store(&refreshable[i]->address, reserved[i]);
> 
> Should not need to perform AtomicAccess because we hold the heap lock here.

When we store a region to alloc region array, AtomicAccess::store will ensure other threads can see it immediately, but the read after holding heap lock has been updated to not AtomicAccess::load.

> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 3045:
> 
>> 3043: }
>> 3044: 
>> 3045: int ShenandoahFreeSet::reserve_alloc_regions(ShenandoahFreeSetPartitionId partition, int regions_to_reserve, size_t min_free_words, ShenandoahHeapRegion** reserved_regions) {
> 
> I request that we not enforce min_free_words when reserving allocation regions.  This defeats the purpose of allocation bias.  The objective is to consume fragmented memory early in the GC cycle (when we have more mitigation options if an allocation request ever fails).  Note that every region that is in any partition has at least PLAB::min_size() available memory.
> 
> By requiring that MUTATOR regions have PLAB::max_size() words, we are forcing ourselves to never consume the fragmented memory regions.  (Towards the end of GC, when memory is in short supply, we will be unable to find directly allocatable MUTATOR regions.  This will force ourselves to obtain the heap lock for every allocation.  And these allocations will be inefficient because the remaining memory is highly fragmented.)

Thanks, I have updated the PR to pass PLAB::min_size() when reserving allocation regions, basically any region with more than PLAB::min_size() can be reserved as shared alloc region, which is same behavior as you are suggesting.

But I will keep the argument   `min_free_words` just in case want to change the behavior in future, if a region has very small amount of memory to barely fit one smallest TLAB/GCLAB, most of allocation in the region may not succeed, we may want to avoid putting regions which are almost ready to retire into CAS allocator.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26171#discussion_r2696536658
PR Review Comment: https://git.openjdk.org/jdk/pull/26171#discussion_r2696541519
PR Review Comment: https://git.openjdk.org/jdk/pull/26171#discussion_r2696534165