RFR: 8231672: Simplify the reference processing parallelization framework [v5]
Leo Korinth
lkorinth at openjdk.java.net
Tue Apr 27 22:10:13 UTC 2021
On Tue, 27 Apr 2021 15:39:02 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
>> 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.
I inlined it.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2782
More information about the hotspot-gc-dev
mailing list