RFR: 8360206: Refactor ReferenceProcessor::balance_queues [v2]

[Kim Barrett]

On Thu, 26 Jun 2025 09:35:40 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> src/hotspot/share/gc/shared/referenceProcessor.cpp line 644:
>> 
>>> 642:     size_t remaining_to_move = move_all
>>> 643:       ? from_len
>>> 644:       : (from_len > avg_refs ? from_len - avg_refs : 0);
>> 
>> `move_all` is (now) only used in the immediately following complicated
>> conditional expression. I think this would be more readable as something like
>> 
>>   // If from_idx >= _num_queues, move all.
>>   // Otherwise, only move excess above avg_refs.
>>   size_t keep = 0;
>>   if (from_idx < _num_queues) {
>>     keep = MIN2(from_len, avg_refs);
>>   }
>>   size_t remaining_to_move = from_len - keep;
>
> I don't think so.
> 
> Your suggested comment:
> 
> 
> // If from_idx >= _num_queues, move all.
> // Otherwise, only move excess above avg_refs.
> 
> can almost directly map to the actual code:
> 
> 
>     size_t remaining_to_move = move_all
>       ? from_len
>       : (from_len > avg_refs ? from_len - avg_refs : 0);
> 
> 
> OTOH, the suggested snippet of using `keep` requires some thinking to connect the dots, IMO.

I find nested ternary conditional operators hard to parse, and I seem to not
be alone in that. Some style guides recommend against ternary conditional
operators at all, though I'm not of that camp. Others allow them but call out
nesting them as one kind of problematically complex form, which I agree with.

Perhaps this?


size_t remaining_to_move = move_all
    ? from_len
    : (from_len - MIN2(from_len, avg_refs));

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25930#discussion_r2172338348