RFR: 8231672: Simplify the reference processing parallelization framework [v5]

[Albert Mingkun Yang]

On Tue, 27 Apr 2021 13:11:38 GMT, Leo Korinth <lkorinth at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/referenceProcessor.hpp line 631:
>> 
>>> 629:   uint index(uint id) const {
>>> 630:     return (_tm == RefProcThreadModel::Single) ? 0 : id;
>>> 631:   }
>> 
>> This method is only used by G1; I wonder if it's possible to not include it in the API of `RefProcProxyTask`.
>
> I could --- and maybe should --- but then I need to supply `_tm` as an argument, further I need to find a logical place to put the function and the declaration, and if I add it to `referenceProcessor.?pp` I have not made it less generic. If I add it to a G1 file, it seems to me arbitrary which to use. If you find a nice place to put it, I will move it, otherwise I will keep it where it is. I will not create a new file for the function and would in that case prefer to inline expand the code.

Since the body of `uint index(uint id) const` is just one line, I was thinking the caller can just inline it. Taking `G1FullGCRefProcProxyTask` as an example.

before

G1FullKeepAliveClosure keep_alive(_collector.marker(index(worker_id)));
G1FollowStackClosure* complete_gc = _collector.marker(index(worker_id))->stack_closure();


after

auto index = _tm == RefProcThreadModel::Single ? 0 : worker_id;
auto marker = _collector.marker(index);
G1FullKeepAliveClosure keep_alive(marker);
G1FollowStackClosure* complete_gc = marker->stack_closure();


My concern with including `uint index(uint id) const` in the API of `RefProcProxyTask` is that its name/implementation is too generic. In the context of `RefProcProxyTask`, it's not obvious what `id` is, what `index` should return, and why `(_tm == RefProcThreadModel::Single) ? 0 : id;` is correct.

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

PR: https://git.openjdk.java.net/jdk/pull/2782