RFR: 8314329: AgeTable: add is_clear() & allocation spec, and relax assert to allow use of 0-index slot [v2]
Albert Mingkun Yang
ayang at openjdk.org
Fri Jan 19 19:01:28 UTC 2024
On Fri, 19 Jan 2024 18:30:52 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:
>> src/hotspot/share/gc/shared/ageTable.hpp line 57:
>>
>>> 55: void clear();
>>> 56: // check whether it's clear
>>> 57: bool is_clear() PRODUCT_RETURN0;
>>
>> The name seems a "proper" API; why is it only for non-product? Is it because it's for verification only? What's its (intended) use case?
>
> 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.)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17470#discussion_r1459566161
More information about the hotspot-gc-dev
mailing list