RFR: Usage tracking cleanup

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


On Tue, 18 Apr 2023 20:44:07 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> There are many nuances to tracing memory utilization. Shenandoah track's usage, waste by humongous objects, padding for promotion LABs alignment and all this is also tracked by generation, the heap and feeds into the heuristics and the pacer. The code to update all of these values and route them to the right places was spread across the allocation call stack. This change consolidates all of the logic into one method, invoked near the end of the allocation.
>
> 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.

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

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


More information about the shenandoah-dev mailing list