RFR: Mixed evacuation [v6]

[Kelvin Nilsen]

On Wed, 21 Apr 2021 11:05:52 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Change definition of CollectionThresholdGarbagePercent
>>   
>>   This is now defined to equal ShenandoahGarbageThreshold, which seems to have
>>   a default value of 25.  The effect on running workloads is to choose more
>>   regions for the collection set than was observed with the previous
>>   configuration.
>>   
>>   Also addressed several improvements in white space and comments.
>>   
>>   The code now runs tier1 and hotspot_gc_shenandoah without regressions.  It
>>   also succsessfully runs an Extremem stress test up until the point of
>>   failure due to triggering of full GC (after 113 completed GC passes, including
>>   two old-gen GC passes).
>
> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 67:
> 
>> 65:   // collection set selection.  Here, we'd rather fall back to this contingent behavior than force a full STW
>> 66:   // collection.
>> 67: #define PROMOTION_BUDGET_BYTES (ShenandoahHeapRegion::region_size_bytes() / 2)
> 
> I'd put macro definitions at the top of the file, right below the includes.

replaced with C++ constant in next commit.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 220:
> 
>> 218:   // future, this threshold percentage may be specified on the command
>> 219:   // line or preferrably determined by dynamic heuristics.
>> 220: #define CollectionThresholdGarbagePercent ShenandoahGarbageThreshold
> 
> Same as above: please place define at top of file.

replaced with C++ constant in next commit.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 249:
> 
>> 247:   _old_coalesce_and_fill_candidates = 0;
>> 248: 
>> 249: #undef CollectionThresholdGarbagePercent
> 
> I don't think the constants need to be undef-ed when defined at top of file. Alternatively, you may consider making them true C++ constants.

Replaced with C++ constant in next commit.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahPassiveOldHeuristics.cpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2018, 2019, 2021, Red Hat, Inc. All rights reserved.
> 
> Same as above, only use first and last years.

Fixed in next commit.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahPassiveOldHeuristics.hpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2018, 2019, 2021, Red Hat, Inc. All rights reserved.
> 
> Same as above.

Fixed in next commit.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahStaticHeuristics.hpp line 32:
> 
>> 30: // TODO: Make this a subclass of ShenandoahHeuristics and create a "tailored" copy of this class to be subclass of
>> 31: // ShenandoahOldHeuristics.  Fix the constructor implementation to invoke ShenandoahHeuristics() superclass constructor.
>> 32: // Change ShenandoahMode::initialize_old_heuristics() to return the subclass of ShenandoahOldHeuristics.
> 
> Is this comment still valid? You seem to propose a ShenandoahStaticOldHeuristics too.

Oops.  Forgot to remove that.  Removed in next commit.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahStaticOldHeuristics.cpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2018, 2019, 2021, Red Hat, Inc. All rights reserved.
> 
> Same as above.

Fixed in next commit.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahStaticOldHeuristics.hpp line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2018, 2019, 2021, Red Hat, Inc. All rights reserved.
> 
> Same as above.

Fixed in next commit.

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

PR: https://git.openjdk.java.net/shenandoah/pull/29