RFR: 8311883: [Genshen] Adaptive tenuring threshold

[Y. Srinivas Ramakrishna]

On Thu, 29 Jun 2023 23:31:59 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahEvacTracker.cpp line 50:
>> 
>>> 48:   _bytes_completed += bytes;
>>> 49:   if (_generational && GenShenCensusAtEvac && young_gen) {
>>> 50:     assert(age > 0, "Error");
>> 
>> Check that age is recorded _after_ incrementing at evacuation.
>
> Yes, it is on the path where we evacuate. It isn't for the case where it's on the marking path. (This is of course the evacuation path here.)

It turns out that the assertion `(age > 0)` is still too strong because it might not be an aging cycle, even though we might be evacuating age 0 objects (without, in this case, incrementing their age).

>> src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp line 472:
>> 
>>> 470:       _worker_id(worker_id) {
>>> 471:     if (_heap->mode()->is_generational()) {
>>> 472:       _tenuring_threshold = _heap->age_census()->tenuring_threshold();
>> 
>> Is this updated in the most recent stop-world marking, or is it using the one from the most recent concurrent collection? Could an interrupted concurrent marking cause a bogus census to be used here?
>> 
>> Partial censuses should be expunged.
>
> It is true that cancellations could lead to bogus partial censuses being used. I'll locate the right place in the cancellation path(s) where partial censuses will  need to be expunged. Essentially, these would be the points where concurrent marking is canceled (when censuses are conducted during the marking phase) and where concurrent evacuation is canceled (when censuses are conducted during the evacuation phase). All other cancellations are fine because the census data will be valid despite cancellation in those cases.

In the event of degeneration, if a census-collection phase is interrupted mid-stream and restarted afresh, the partial census will need to be expunged. It'll then be replaced by the new census (in e.g. degenerate GC's mark or evac). In the case of a degenerate STW GC being cancelled and degenerating further to a full STW GC, during _its_ census-collection phase the partial census will be expunged. Any completed censuses from any census taking phase that persisted will be used as before. Currently, all calculations are independent of the (start and end) time(s) of the (completed) censuses being used, but a future enhancement might potentially consider a temporal dimension. We'll capture the times in the census today, but those are not used by the algorithm today.

>> src/hotspot/share/gc/shenandoah/shenandoahHeap.inline.hpp line 484:
>> 
>>> 482:     // If copying to the old generation, we don't care about recording
>>> 483:     // object age in the census stats.
>>> 484:     _evac_tracker->end_evacuation(thread, size * HeapWordSize, young_gen, young_gen ? ShenandoahHeap::get_object_age(copy_val) : 0);
>> 
>> Revisit this to see why we record age 0 for the case of old gen objects.
>
> To answer the question, "why 0?" : it was because I didn't want to extract the age for the old_gen case, because it isn't used in that case (the census doesn't record old gen object ages). I'll see about cleaning this up a bit.

Cleaned this up to avoid this pattern. Hopefully, it reads a bit cleaner and less clever (and error-prone) now.

>> src/hotspot/share/gc/shenandoah/shenandoah_globals.hpp line 47:
>> 
>>> 45:           "Floor for adaptive tenuring threshold")                          \
>>> 46:                                                                             \
>>> 47:   product(uintx, GenShenMaxTenuringThreshold, 16, EXPERIMENTAL,             \
>> 
>> Why not 15?
>
> We'll need to discuss this. Allowing this to be 16 (i.e. 1 greater than max_age) allows for a potential "never tenure" behavior. Otherwise, age 15 objects would always become eligible for tenuring. I think allowing it to be above 15 (i.e. allowing 16, and allowing objects to stay in young till and including age 15) would seem to make sense.
> 
> I'll set the default to 15 (thus never allowing objects that , but allow users to set it to 16, simulating "never tenure", if both min and max are set to 16.

Provide guidance here on how to use these min and max values to obtain different behavior, while picking sensible defaults (i.e. 0 and 15).

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/289#discussion_r1250189453
PR Review Comment: https://git.openjdk.org/shenandoah/pull/289#discussion_r1260019548
PR Review Comment: https://git.openjdk.org/shenandoah/pull/289#discussion_r1248354258
PR Review Comment: https://git.openjdk.org/shenandoah/pull/289#discussion_r1251388564