RFR: 8255984: Shenandoah: "adaptive" heuristic is prone to missing load spikes
github.com+71722661+earthling-amzn at openjdk.java.net
Thu Nov 12 22:25:57 UTC 2020
On Wed, 11 Nov 2020 08:12:50 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:
>> This change adds a "reactive" heuristic for triggering concurrent GC cycles.
>> The reactive heuristic maintains a margin of error and an allocation spike detection mechanism to trigger cycles somewhat more aggressively than the 'adaptive' heuristic. This heuristic 'reacts' to the outcome of GC cycles by adjusting the sensitivity of the triggers.
>> JBS ticket is here: https://bugs.openjdk.java.net/browse/JDK-8255984
>> The "adaptive" heuristic remains the default.
>> Steps to reproduce and test will follow shortly (there are no new jtreg test failures for Shenandoah with this change).
> src/hotspot/share/gc/shenandoah/heuristics/shenandoahAdaptiveHeuristics.cpp line 212:
>> 210: ShenandoahAdaptiveHeuristics *heuristic = const_cast<ShenandoahAdaptiveHeuristics *>(this);
>> 211: heuristic->_allocation_rate.sample(bytes_allocated_since_gc_start);
>> 212: heuristic->_last_trigger = OTHER;
> Shouldn't `_last_trigger` updates be near `return true;` in this method? I.e. we should not probably overwrite `_last_trigger` if nothing was actually triggered? This probably affects the super-call to `ShenandoahHeuristics::should_start_gc()` as well?
Would it make more sense if I `OTHER` were `NONE`? The intention here is to only adjust the allocation rate or spike detection parameters if they triggered the cycle. Here we reset the `_last_trigger` in one place without having to touch all the actual decision points. As it stands, once a gc cycle is started, the control thread won't call back into this method until the cycle is complete.
More information about the hotspot-gc-dev