RFR: 8365956: GenShen: Adaptive tenuring threshold algorithm may raise threshold prematurely [v3]

William Kemper wkemper at openjdk.org
Thu Aug 28 17:26:43 UTC 2025


On Thu, 28 Aug 2025 00:59:08 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahAgeCensus.hpp line 181:
>> 
>>> 179:   // Return true if this age is above the tenuring threshold.
>>> 180:   bool is_tenurable(uint age) const {
>>> 181:     return age > tenuring_threshold();
>> 
>> Another key change. This prevents the algorithm from seeing higher mortality rates caused by promotions in the cohort at tenuring age.
>
> The strict ">" says objects of age strictly greater than the computed tenuring threshold are tenurable, not those of that age. This might make sense if it were used at all the places where tenuring decisions were being made during evacuation. Was that the intention?
> 
> That doesn't seem to be the case -- there are pre-existing places in the code where a raw comparison with the tenuring threshold are made that don't do the strict check. Might be a good idea to go over all such spots and use this method (and inline it for performance?). It would then make `tenuring_threshold()` a private accessor, so the official API would be `is_tenurable(age)`, and no raw comparisons w/`tenuring_threshold()` that might become mutually inconsistent.
> 
> See raw comparisons in:
> 1. `ShenandoahYoungHeuristics::choose_young_collection_set`
> 2. `ShenandoahGenerationalEvacuationTask::maybe_promote_region`
> 3. `ShenandoahGenerationalHeuristics::choose_collection_set`
> 4. `ShenandoahGenerationalHeuristics::add_preselected_regions_to_collection_set`
> 5. `ShenandoahGlobalHeuristics::choose_global_collection_set`
> 6. `ShenandoahCollectionSet::add_region`
> 7. etc... there are several more.

Yes, that the intention was to make the comparison strictly greater than (to prevent the adaptive tenuring algorithm from looking at cohorts that may have been promoted). I'll make the changes you suggested. Methods defined in headers are implicitly inline.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26906#discussion_r2308056678


More information about the hotspot-gc-dev mailing list