RFR: 8377048: Shenandoah: shenandoahLock related improvments [v3]

[Xiaolong Peng]

On Tue, 3 Feb 2026 17:32:57 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> Xiaolong Peng has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Locker class should assume that lock instance is never be nullptr
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 790:
> 
>> 788:   // inconsistent data: available may not equal capacity - used because the intermediate states of any "atomic"
>> 789:   // locked action can be seen by these unlocked functions.
>> 790:   inline size_t capacity_holding_lock() const {
> 
> It looks like you're adding overhead to these calls here.  Previously, we were able to ask for capacity() and used() and available() without needing to pay the cost of acquiring a lock.  Now, we always have to acquire the lock().  Have you measured impact?

I didn't run test for it yet, but for non-debug build, the capacity/used/available are not frequently read, the overhead should have very minor impact(it may still compete with thread doing rebuild). 
In debug build, because of the asserts need to read the accountings much more often, it should have a small performance impact.

The change is really about correctness, these filed may be be updated with rebuild lock being hold, but can be read w/o acquiring the rebuild lock, so a thread reading them w/o the rebuild lock may see stale value.

As a part of the change of JDK-8373371, I am updating how the heap usage accountings are updated:
1. All changes to heap usage accountings must be done with holding heap lock and heap usage accounting lock(rebuild lock will be renamed to usage accounting lock),
2. All reads must be done under heap usage accounting lock, no need to hold heap lock. 
3. Computation of total/young/old accountings will be done in the read path under heap usage accounting lock.

Here are the examples of young_used(), old_used(), global_used() with deferred re-computation:

  // Return bytes used by young
  inline size_t young_used() {
    ShenandoahHeapUsageAccountingLocker locker(usage_accounting_lock());
    return _partitions.used_by(ShenandoahFreeSetPartitionId::Mutator) +
      _partitions.used_by(ShenandoahFreeSetPartitionId::Collector);
  }

  // Return bytes used by old
  inline size_t old_used() {
    ShenandoahHeapUsageAccountingLocker locker(usage_accounting_lock());
    return _partitions.used_by(ShenandoahFreeSetPartitionId::OldCollector);
  }

  // Return bytes used by global
  inline size_t global_used() {
    ShenandoahHeapUsageAccountingLocker locker(usage_accounting_lock());
    return young_used() + old_used();
  }


ShenandoahHeapUsageAccountingLock is a reentrant lock, global_used acquires the lock first, so it will have stable snapshot of young_used and old old_used when it calculate the global_used. 

Reentrance of the ShenandoahHeapUsageAccountingLock is very cheap since the current thread already holds it, it just checks the owner of the lock and increase the counter.

I have pushed this change to gitfarm pipeline, I'm not really expecting any performance regression form this, I'll check the result tomorrow.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29543#discussion_r2762629599