RFR: 8364423: G1: Refactor G1UpdateRegionLivenessAndSelectForRebuildTask

Albert Mingkun Yang ayang at openjdk.org
Thu Jul 31 15:37:55 UTC 2025


On Thu, 31 Jul 2025 15:23:13 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 1262:
>> 
>>> 1260:       }
>>> 1261:       _cm->update_top_at_rebuild_start(hr);
>>> 1262:     }
>> 
>> I feel this logic is too caller-specific -- reading its name `update_live_region` doesn't reveal much what this method should do/deliver.
>
> Since it is the same, and the only per-region work to do for all callers, not sure what is caller-specific here. Maybe change the name to something like `update_for_rebuild` is more appropriate? Or just leave it as is until more common work needs to be done?

Usually, it's the method spec that determines its impl, because a method doesn't know all the callers. However, give its name + signature, it's unclear what the impl should be -- instead, it's determined by its caller context (select-for-rebuild), hence, caller-specific.

> Or just leave it as is until more common work needs to be done?

Maybe by then some common logic can be extracted into a self-contained method.

YMMV.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26574#discussion_r2245738251


More information about the hotspot-gc-dev mailing list