RFR: 8328626: GenShen: Combine old generation surplus/deficit fields into a single balance field [v2]

Kelvin Nilsen kdnilsen at openjdk.org
Fri Mar 22 22:47:41 UTC 2024


On Fri, 22 Mar 2024 17:44:02 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> This is a follow up to https://github.com/openjdk/shenandoah/pull/406.
>
> William Kemper has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix zero build
>  - Fix comments

Marked as reviewed by kdnilsen (Committer).

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 1185:

> 1183: void ShenandoahFreeSet::compute_young_and_old_reserves(size_t young_cset_regions, size_t old_cset_regions,
> 1184:                                                        bool have_evacuation_reserves, size_t* young_reserve_result,
> 1185:                                                        size_t* old_reserve_result) const {

Might read more "naturally" if args declared as:

size_t& young_reserve_result, size_t& old_reserve_result

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 1220:

> 1218:   // All allocations taken from the old collector set are performed by GC, generally using PLABs for both
> 1219:   // promotions and evacuations.  The partition between which old memory is reserved for evacuation and
> 1220:   // which is reserved for promotion is enforced using thread-local variables that prescribe intentons for

typo: intentions

src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.cpp line 128:

> 126: 
> 127:   if (old_region_balance > 0) {
> 128:     const auto old_region_surplus = checked_cast<size_t>(old_region_balance);

Just curious: what happens with a checked_cast<size_t> if old_region_balance < 0.  (I see that's impossible here, but what is the point of declaring checked_cast?)

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

PR Review: https://git.openjdk.org/shenandoah/pull/410#pullrequestreview-1955894108
PR Review Comment: https://git.openjdk.org/shenandoah/pull/410#discussion_r1536285718
PR Review Comment: https://git.openjdk.org/shenandoah/pull/410#discussion_r1536284815
PR Review Comment: https://git.openjdk.org/shenandoah/pull/410#discussion_r1536286804


More information about the shenandoah-dev mailing list