RFR: Fix satb barrier for object array copy [v2]
William Kemper
wkemper at openjdk.org
Thu Mar 30 23:32:32 UTC 2023
On Thu, 30 Mar 2023 23:27:57 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:
>> The satb barrier for overwritten arrays residing in old-gen must be unconditional during young GC.
>>
>> During old GC, the satb barrier is only needed for overwritten old-gen arrays if the array is below TAMS within its old-gen region.
>
> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>
> Fix typo in comment
There is a typo in the comments. Suggest using a consistent test for "array is below TAMS".
src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp line 461:
> 459: assert(_heap->mode()->is_generational(), "Invariant");
> 460: ShenandoahHeapRegion* r = _heap->heap_region_containing(array_addr);
> 461: if (r->is_old() && (array_addr < _heap->marking_context()->top_at_mark_start(r))) {
Why not use `!_heap->marking_context()->allocated_after_mark_start(array_addr)` as other branches do? Or replace other `!_heap->marking_context()->allocated_after_mark_start(array_addr)` with `array_addr < _heap->marking_context()->top_at_mark_start(r))` to avoid a second region look up?
src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp line 476:
> 474: } else {
> 475: T* array = src;
> 476: if (!_heap->marking_context()->allocated_after_mark_start(reinterpret_cast<HeapWord*>(array))) {
Pull `array_addr` up and replace `reinterpret_cast<HeapWord*>(array)` with `array_addr`?
-------------
PR Review: https://git.openjdk.org/shenandoah/pull/237#pullrequestreview-1366064653
PR Review Comment: https://git.openjdk.org/shenandoah/pull/237#discussion_r1153877907
PR Review Comment: https://git.openjdk.org/shenandoah/pull/237#discussion_r1153877631
More information about the shenandoah-dev
mailing list