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

[Thomas Schatzl]

On Thu, 4 Mar 2021 15:05:12 GMT, Leo Korinth <lkorinth at openjdk.org> wrote:

>> src/hotspot/share/gc/parallel/psScavenge.cpp line 183:
>> 
>>> 181:  private:
>>> 182:   PSPromotionManager* _promotion_manager;
>>> 183:   TaskTerminator* _maybe_terminator;
>> 
>> What does the `_maybe` in `_maybe_terminator` indicate? Just naming it `_terminator` seems as fine as a `nullptr` seems like a valid value.
>> It is explained in `PSRefProcClosureContext`, but it seems too much to add this everywhere where a pointer could be a valid `nullptr`.
>
> Maybe indicates (Just a valid pointer) or (Nothing nullptr), optional might be a better prefix? 
> 
> I would agree to remove the prefix if we would consistently use references where nullptr is not a valid state. If we are going to use pointers instead of references because it visualizes that we take the address at call site (something I disagree with), I think adding `maybe` or `optional` is a (visual) safeguard for not de-referencing the pointer without verifying it can safely be de-referenced.
> 
> I will remove the prefix if you do not buy my explanation.

Not really seeing the advantage: if you want to make sure that something isn't `nullptr` when it's used, put an assert before the use, then it should be clear (and runtime-checked) that it's not valid; even if it were `nullptr`, testing should have found (i.e. exercised that path) that anyway and failed fast (by dereferencing that null value).

A reviewer needs to look if values may actually be `nullptr` in that code path anyway regardless of the name.

Maybe someone else likes this better though, so no opinion here.

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

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