RFR: 8327997: G1: Move G1ScanClosureBase::reference_iteration_mode to subclass [v2]

[Albert Mingkun Yang]

On Thu, 14 Mar 2024 09:34:24 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Albert Mingkun Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
>> 
>>  - drop-comment
>>  - g1-card-scan-mode
>
> src/hotspot/share/gc/g1/g1OopClosures.hpp line 69:
> 
>> 67:     G1ScanClosureBase(g1h, pss), _heap_roots_found(heap_roots_found) { }
>> 68: 
>> 69:   // Because this closure is applied on pointers residing outside the
> 
> I think the comment is okay, but maybe should put more emphasis on that specifying this iteration mode allows _the compiler_ to (more easily) compile out the check in `try_discover`.
> 
> Also since this isn't the only place this (and similar) optimizations are made elsewhere too using this getter, I kind of agree with @lgxbslgx that the impact of the selection should probably be documented at the enum level (the move of the override into the leaf class here is fine with me).
> 
> Just something like "// Selection of one or the other option helps the compiler to remove unnecessary, known beforehand behavior for a given oop closure at compile time" seems sufficient to me there instead of this fairly long comment.
> The exact changes can be easily gathered by searching the usages of the enum.

I dropped the comment. This optimization, afaics, is only used/implemented in G1. Moving the comment to the enum level can be confusing, since its impact might not be visible (or even positive) for other collectors. (I inspected the generated asm of `g1Remset` with and without this optimization and they are indeed different -- the one *without* the optimization is shorter (in terms of LOC), but it's unclear which one is faster.)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18244#discussion_r1524572852