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