RFR: 8369048: GenShen: Defer ShenFreeSet::available() during rebuild
William Kemper
wkemper at openjdk.org
Fri Oct 17 17:43:15 UTC 2025
On Thu, 2 Oct 2025 17:58:48 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:
> This code introduces a new rebuild-freeset lock for purposes of coordinating the freeset rebuild activities and queries as to memory available for allocation in the mutator partition.
>
> This addresses a problem that results if available memory is probed while we are rebuilding the freeset.
>
> Rather than using the existing global heap lock to synchronize these activities, a new more narrowly scoped lock is introduced. This allows the available memory to be probed even when other activities hold the global heap lock for reasons other than rebuilding the freeset, such as when they are allocating memory. It is known that the global heap lock is heavily contended for certain workloads, and using this new lock avoids adding to contention for the global heap lock.
If I understand correctly, the general issue is that `available` is not accurate when the freeset is being rebuilt. There are three solutions tested:
1. Existing code, returns a sentinel value (`SIZE_MAX`) during freeset rebuilt
2. Return the last known value of `available` during the rebuild (this appears to cause more aggressive heuristics)
3. Block threads that call `available` during rebuild
As it stands, only the regulator thread (which is evaluating heuristics for genshen) will call `available` during a free set rebuild (though this may change in the future). With the first solution, it seems we would have the heuristics believe there is much more memory available than there actually is. This would risk the heuristic not triggering when it should?
It makes sense that option `2` would trigger more GCs than option `1`, but it seems the risk of triggering too late would be lower here. Option `3` might also delay triggering, but at least the heuristic would base the trigger decision on an accurate accounting of available memory.
If we go with the third option, I think we should move the lock management into the freeset and not have to change existing callers.
void ShenandoahFreeSet::prepare_to_rebuild(...) {
_lock.lock();
// do preparation
// ...
}
void ShenandoahFreeSet::finish_rebuild(...) {
// finish rebuild
// ...
_lock.unlock();
}
Could we also now remove the sentinel value (`FreeSetUnderConstruction`)?
-------------
Changes requested by wkemper (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/27612#pullrequestreview-3351287418
More information about the shenandoah-dev
mailing list