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