On Thu, 22 Aug 2024 17:12:09 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:

>> Roman Kennke has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Remove hashcode leftovers from SA
>
> src/hotspot/share/gc/serial/defNewGeneration.cpp line 707:
> 
>> 705:       } else if (obj->is_forwarded()) {
>> 706:         // To restore the klass-bits in the header.
>> 707:         obj->forward_safe_init_mark();
> 
> I wonder if not modifying successful-forwarded objs is cleaner. Sth like:
> 
> 
> reset_self_forwarded_in_space(space) {
>   cur = space->bottom();
>   top = space->top();
> 
>   while (cur < top) {
>     obj = cast_to_oop(cur);
> 
>     if (obj->is_self_forwarded()) {
>       obj->unset_self_forwarded();
>       obj_size = obj->size();
>     } else {
>       assert(obj->is_forwarded(), "inv");
>       obj_size = obj->forwardee()->size();
>     }
> 
>     cur += obj_size;
>   }
> }
> 
> reset_self_forwarded_in_space(eden());
> reset_self_forwarded_in_space(from());

I was thinking the same, but there's a problem with that. If we get a promotion 
failure in the young gen, we are leaving the dead objects marked as forwarded. 
Then when the Full GC scans these regions with dead objects it will mistakenly 
think that they have been marked alive because `is_forwarded() == 
is_gc_marked()`. The code in `phase2_calculate_new_addr` will then break when 
it looks for `is_gc_marked` objects.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1738433303

Reply via email to