RFR: 8341099: GenShen: assert(HAS_FWD == _heap->has_forwarded_objects()) failed: Forwarded object status is sane

[William Kemper]

On Thu, 3 Oct 2024 23:18:41 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> Evacuations may occur during young or mixed collection cycles. When it is a young cycle during old marking, this barrier may be called to both evacuate cset objects and again to enqueue the targets of the copy (if they are in the old generation). If it is a mixed collection, then old marking will _not_ be in progress and this barrier will not be called to enqueue anything.
>
> src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp line 406:
> 
>> 404:         shenandoah_assert_forwarded_except(elem_ptr, obj, _heap->cancelled_gc());
>> 405:         ShenandoahHeap::atomic_update_oop(fwd, elem_ptr, o);
>> 406:       }
> 
> I think the reason we are removing this assignment here is because we will never be at this line of code if (ENQUEUE && !ctx->is_marked_strong_or_old(obj)).  In other words, if we are evacuating this object, then this object is not in old or this object is in old and we are not marking old.  I'd feel a little more comfortable with this code change if there were at minimum a comment here to explain this, or perhaps even better an assert.
> 
> assert(!ENQUEUE || !is_old(obj), "Do not evacuate old while old marking is active")
> 
> something like this...

Yes, I added an assert at the top of the function to this effect.

  // This function cannot be called to handle marking and evacuation at the same time (they operate on
  // different sides of the copy).
  assert((HAS_FWD || EVAC) != ENQUEUE, "Cannot evacuate and mark both sides of copy.");

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/507#discussion_r1786938578