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

[Albert Mingkun Yang]

On Wed, 25 Jun 2025 22:51:45 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Albert Mingkun Yang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:
>> 
>>  - Merge branch 'master' into ref-refactor
>>  - ref-refactor
>
> src/hotspot/share/gc/shared/referenceProcessor.cpp line 638:
> 
>> 636:     if (from_len == 0) {
>> 637:       // empty
>> 638:       continue;
> 
> Kind of waffling over this, but I'm not sure this is worthwhile. The result falls out of the 
> `remaining_to_move` calculation and use, and it's kind of cluttering.

I thought it's clearer to handle the empty-case explicitly. No strong feelings either way; removed.

> 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.

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

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