RFR: Usage tracking cleanup

William Kemper wkemper at openjdk.org
Tue Apr 18 23:54:18 UTC 2023


On Tue, 18 Apr 2023 23:48:57 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 249:
>> 
>>> 247: }
>>> 248: 
>>> 249: size_t get_usable_free_words(size_t free) {
>> 
>> Please write a block comment:
>> 
>> // This work method takes an argument corresponding to the number of bytes
>> // free in a region, and returns the largest amount in heapwords that can be allocated
>> // such that both of the following conditions are satisfied:
>> //
>> // 1. it is a multiple of card size
>> // 2. any remaining shard may be filled with a filler object
>> //
>> // The idea is that the allocation starts and ends at card boundaries. Because
>> // a region ('s end) is card-aligned, the remainder shard that must be filled is
>> // at the start of the free space.
>> //
>> // This is merely a helper method to use for the purpose of such a calculation.
>> 
>> 
>> I don't like the scale/unit translation that this method does. You already hint at that via naming the method `get_usable_free_words`. Repeating this in the documentation comment above helps as well. Finally, we can ensure that
>> the user sees this up front by calling the argument "free_bytes" to further underline this point.
>
> Yes, I agree. I would like something like a strong typedef here to prevent mixing bytes and words in the same expression. If you look closely at the [original code](https://github.com/openjdk/shenandoah/pull/260/files#diff-46de849c458b5bfca09d1c8134e9d71d131faac923b39d44db2fe27d168941aeL370), the path for in-elastic TLABs mixed up words and bytes.

I wrote it this way to reduce duplication in the callers.

-------------

PR Review Comment: https://git.openjdk.org/shenandoah/pull/260#discussion_r1170672290


More information about the shenandoah-dev mailing list