RFR: 8333229: Parallel: Rename ParMarkBitMap::_region_start to _heap_start

Thomas Schatzl tschatzl at openjdk.org
Mon Jun 3 11:14:03 UTC 2024


On Wed, 29 May 2024 20:08:51 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> Trivial renaming to better capture that this class corresponds to the whole heap mem-region.

Lgtm and trivial. Please remove that imo confusing comment before pushing.

src/hotspot/share/gc/parallel/psParallelCompact.hpp line 523:

> 521: {
> 522:   assert(addr >= _heap_start, "bad addr");
> 523:   // would mistakenly return 0 for _heap_end

(pre-existing): I think this comment should be removed as it is really confusing in the current context because it refers to deleted code.

It seems that previously the next assert used `<=` instead of `<` which could cause the mentioned problem. However without context (like "Using <= would mistakenly...") this comment makes no sense. So I recommend removing this comment because even such a fixed comment would not help a lot imo.

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

Marked as reviewed by tschatzl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19468#pullrequestreview-2093521928
PR Review Comment: https://git.openjdk.org/jdk/pull/19468#discussion_r1624236199


More information about the hotspot-gc-dev mailing list