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