RFR: Usage tracking cleanup [v2]

Y. Srinivas Ramakrishna ysr at openjdk.org
Thu Apr 20 01:38:39 UTC 2023


On Wed, 19 Apr 2023 22:59:17 GMT, William Kemper <wkemper 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.
>
> William Kemper has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:
> 
>  - Track total heap usage in global generation
>  - Remove unused method: clear_used
>  - Readability improvements
>  - Merge branch 'shenandoah-master' into alloc-tracking-cleanup
>  - Merge branch 'shenandoah-master' into alloc-tracking-cleanup
>  - Remove invalid assert
>  - Merge branch 'shenandoah-master' into alloc-tracking-cleanup
>  - Consolidate usage/allocate/waste accounting
>  - WIP: Oops - report usage in bytes, not words
>  - WIP: Initialize new padding field
>  - ... and 2 more: https://git.openjdk.org/shenandoah/compare/016bf071...99c52914

Thanks for your patience with my review comments! 

This looks good to me. 🚢

src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp line 1336:

> 1334:       if (r->is_old()) {
> 1335:         account_for_region(r, _old_regions, _old_usage, _old_humongous_waste);
> 1336:       } else if (r->is_young()) {

In the non-generational case, the region here will always answer `young` ?

Do we want to `assert(false, ...)` in the else arm at line 1338, like the comment seems to suggest?

src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp line 1363:

> 1361: 
> 1362:     // In generational mode, global usage should be the sum of young and old. This is also true
> 1363:     // for non-generational modes except that there are no old regions.

... so the first addend (or, augend) in each of the arguments below is 0 in the non-generational case? If so, is that worth asserting, just to be super-paranoid?

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

Marked as reviewed by ysr (Author).

PR Review: https://git.openjdk.org/shenandoah/pull/260#pullrequestreview-1392588344
PR Review Comment: https://git.openjdk.org/shenandoah/pull/260#discussion_r1171956904
PR Review Comment: https://git.openjdk.org/shenandoah/pull/260#discussion_r1171953575


More information about the shenandoah-dev mailing list