RFR: Fix off-by-one error when handling pinned regions in a mixed collection

Kelvin Nilsen kdnilsen at openjdk.org
Thu Mar 16 00:57:14 UTC 2023


On Wed, 15 Mar 2023 23:13:38 GMT, William Kemper <wkemper at openjdk.org> wrote:

> This fixes an off-by-one error that manifested when the first mixed collection candidate was pinned.

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 156:

> 154: 
> 155:   // Find pinned regions to the left and move their pointer into a slot
> 156:   // that was pointing at a region that has been added to the cset. We

(maybe augment this comment: "or was holding another pinned region that has already been copied to an unused slot found further to the right of its original position"

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 166:

> 164:     RegionData& skipped = _region_data[search];
> 165:     if (skipped._region->is_pinned()) {
> 166:       RegionData& added_to_cset = _region_data[write_index];

And this variable might be better named "unused_available_slot" rather than "added_to_cset".  In the case that we are overwriting a pinned region pointer, it was not "added_to_cset".

src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.cpp line 186:

> 184:   //   | Start index maintains the pointer to the beginning of the regions we scanned on this cycle.
> 185: #ifdef ASSERT
> 186:   for (size_t check = write_index; check != _start_candidate; --check) {

I see that _start_candidate is initialized to zero when ShenandoahOldHeuristics is instantiated.  What I do not understand is how _start_candidate is reset to zero each time we rebuild _region_data with a new set of mixed evacuation candidates.  Can you provide a comment to explain this?

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

PR: https://git.openjdk.org/shenandoah/pull/226


More information about the shenandoah-dev mailing list