RFR: 8360206: Refactor ReferenceProcessor::balance_queues [v2]
Kim Barrett
kbarrett at openjdk.org
Wed Jun 25 23:01:38 UTC 2025
On Tue, 24 Jun 2025 07:45:15 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 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.
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;
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25930#discussion_r2167773359
PR Review Comment: https://git.openjdk.org/jdk/pull/25930#discussion_r2167769088
More information about the hotspot-gc-dev
mailing list