RFR: 8325671: Shenandoah: Introduce a ShenandoahGenerationType and templatize certain marking closures with it [v4]

[Y. Srinivas Ramakrishna]

On Thu, 15 Feb 2024 13:33:42 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Y. Srinivas Ramakrishna has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
>> 
>>  - Merge branch 'master' into generation_type
>>  - Merge branch 'master' into generation_type
>>  - Missing #include of ShenandoahGenerationType (albeit satisfied transitively via other include).
>>  - Merge branch 'master' into generation_type
>>  - Introduce ShenandoahGenerationType and templatize most closures with it.
>>    The template expands for only the NON_GEN type for the non-generational
>>    version of Shenandoah currently, and will in the future accomodate
>>    Generational Shenandoah.
>
> src/hotspot/share/gc/shenandoah/shenandoahMark.cpp line 92:
> 
>> 90: 
>> 91: template<bool CANCELLABLE, StringDedupMode STRING_DEDUP>
>> 92: void ShenandoahMark::mark_loop(ShenandoahGenerationType generation /* ignored */, uint worker_id, TaskTerminator* terminator, ShenandoahReferenceProcessor *rp, StringDedup::Requests* const req) {
> 
> I think `/* ignored */` is not needed here.
> 
> Also, the argument list line is probably too long. AFAICS, we put the arguments we curry from method arguments to template arguments at very end. See how the method arguments are entering the template arguments in the same order:
> 
> 
> template<bool CANCELLABLE, StringDedupMode STRING_DEDUP>
> void ShenandoahMark::mark_loop(uint worker_id, TaskTerminator* terminator, ShenandoahReferenceProcessor *rp,
>                                ShenandoahGenerationType generation, StringDedup::Requests* const req) ... 
> 
> void ShenandoahMark::mark_loop(uint worker_id, TaskTerminator* terminator, ShenandoahReferenceProcessor *rp,
>                                bool cancellable, StringDedupMode dedup_mode, ShenandoahGenerationType generation, StringDedup::Requests* const req ...

I'll fix the order so it conforms to that convention; thanks for pointing it out!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17815#discussion_r1491360556