On Tue, 8 Oct 2024 10:20:45 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:
>> I was thinking that we should remove the entire: >> >> // Note: If min-fill-size decreases to 1, this whole method becomes >> redundant. >> assert(CollectedHeap::min_fill_size() >= 2, "inv"); >> >> block, since it is now incorrect, guarded by the proper check, and the >> comment is misleading since we now can have a min-fill-size that is 1. > > It's still correct when UCOH is disabled -- therefore, the UCOH check can be > placed at the start without changing any existing logic. (The "rest" of this > method assumes min-fill-size is 2, `assert(CollectedHeap::min_fill_size() == > 2, "inv")`.) > > In this PR, since this method doesn't access UCOH, it can be easily forgotten > to update this method when the UCOH flag is removed eventually -- it's not > obvious to me that `MinObjAlignment >= > checked_cast<int>(CollectedHeap::min_fill_size())` is related to (or can be > affected by) `UCOH` at first glance. > > (I slightly prefer having a `if (UCOH)` inside this method, but considering > this method will be nuked in the long run, any short-time decision is fine by > me, assuming the failing assert is fixed.) I added an assert(!UCOH) in the implementation so that we don't forget it once the UCOH flag becomes obsolete. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20677#discussion_r1791619505
