RFR: 8351091: Shenandoah: global marking context completeness is not accurately maintained [v7]

[Xiaolong Peng]

On Thu, 3 Apr 2025 21:34:06 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Xiaolong Peng has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Can't verify marked object with complete marking after full GC
>
> src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp line 352:
> 
>> 350:     assert(_from_region != nullptr, "must set before work");
>> 351:     assert(_heap->active_generation()->complete_marking_context()->is_marked(p), "must be marked");
>> 352:     assert(!_heap->active_generation()->complete_marking_context()->allocated_after_mark_start(p), "must be truly marked");
> 
> I am probably being a bit pedantic here...
> 
> I would use `gc_generation()` in all code that is executed strictly by GC threads, and `active_generation()` in all code that may possibly be executed by a mutator thread. It seems as if today this code is only executed by GC threads.
> 
> In general, there is no real distinction between these field at times like these (STW pauses) when heap verification is taking place, but from a syntactic hygiene perspective.
> 
> We can otherwise file a ticket to separately clean up any confusion in the use of these fields (and add a dynamic check to prevent creeping confusion). The names aren't super well-chosen, but generally think of `_gc_generation` as the generation that is being GC'd, `_active_generation` as one that mutator threads are aware is being the subject of GC. Any assertions by mutator threads should use the latter and by GC threads the former. The fields are reconciled at STW pauses.

Make sense, I did notice that there is assert `assert(!Thread::current()->is_Java_thread(), "Not allowed");` in `gc_generation()` suggesting that non-Java thread should call  `gc_generation()`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23886#discussion_r2027837825