RFR: 8314329: AgeTable: add is_clear() & allocation spec, and relax assert to allow use of 0-index slot [v2]
Y. Srinivas Ramakrishna
ysr at openjdk.org
Fri Jan 19 21:32:27 UTC 2024
On Fri, 19 Jan 2024 18:58:38 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
>> Thanks for taking a look, Albert.
>>
>> It was indeed intended just for verification/assertion checks.
>>
>> The use case(s) can be found here in the GenShen repo:
>>
>> https://github.com/openjdk/shenandoah/blob/5c6f4171efd250ab47695059814a78d7b672a4bd/src/hotspot/share/gc/shenandoah/shenandoahAgeCensus.cpp#L190
>>
>> and other spots in the same file. They are all for assertion checks.
>>
>> I expect that this will be used only for assertions, whenever it is, hence non-product.
>
> It's not obvious to me that `bool clear = _global_age_table[i]->is_clear();` is for assert-only. Looking at `ShenandoahAgeCensus::is_clear_global`, the whole methods looks like it's real/product logic.
>
>> I expect that this will be used only for assertions, whenever it is, hence non-product.
>
> Then, can its definition be guarded like following? My concern is that this API returns different value depending on whether it is a product build.
>
>
> #ifdef ASSERT // or ifndef product
> bool is_clear();
> #endif
>
>
> (Or, leaving it as an ordinary API seems OK as well, IMO.)
Fair point. I'll fix this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17470#discussion_r1459806631
More information about the hotspot-gc-dev
mailing list