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