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

Kim Barrett kbarrett at openjdk.org
Sat Jun 28 01:17:48 UTC 2025


On Fri, 27 Jun 2025 19:01:38 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Simple refactoring to clean up the inner-loop conditions and control flow. Due to the structural change, the diff is slightly large and may be harder to read than the resulting code. The new version simplifies the logic by using a single `remaining_to_move` counter in the inner loop to track remaining work, and introduces early returns to reduce indentation and improve clarity.
>> 
>> Test: tier1-3
>
> 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 six additional commits since the last revision:
> 
>  - Merge branch 'master' into ref-refactor
>  - review
>  - review
>  - Merge branch 'master' into ref-refactor
>  - Merge branch 'master' into ref-refactor
>  - ref-refactor

There's a small bit of code where @albertnetymk and I have some stylistic
differences, but they aren't so critical as to make me want to block this change.
Other parts of the change seem worthwhile.

src/hotspot/share/gc/shared/referenceProcessor.cpp line 645:

> 643:       remaining_to_move = from_len > avg_refs
> 644:                         ? from_len - avg_refs
> 645:                         : 0;

Continuing the discussion about this snippet.

I think that use of the ternary operator here looks weird to me, being
combined with the if-statement that way. I might write

size_t remaining_to_move = from_len;
if (from_idx < _num_queues) {
  if (from_len <= avg_refs) {
    remaining_to_move = 0;
  } else {
    remaining_to_move = from_len - avg_refs;
  }
}

or more likely

size_t remaining_to_move = from_len;
if (from_idx < _num_queues) {
  remaining_to_move -= MIN2(from_len, avg_refs);
}

The calculation in line 3 is an ancient and venerable idiom for "subtract,
clipping to non-negative".

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

Marked as reviewed by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25930#pullrequestreview-2968296823
PR Review Comment: https://git.openjdk.org/jdk/pull/25930#discussion_r2173070770


More information about the hotspot-gc-dev mailing list