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