RFR: Usage tracking cleanup
William Kemper
wkemper at openjdk.org
Wed Apr 19 00:10:37 UTC 2023
On Tue, 18 Apr 2023 21:13:16 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 272:
>
>> 270: assert(size % CardTable::card_size_in_words() == 0, "size must be multiple of card table size, was " SIZE_FORMAT, size);
>> 271:
>> 272: HeapWord* result = r->allocate_aligned(size, req, CardTable::card_size());
>
> Note that `allocate_aligned` takes a bunch of safety measures that are, in this particular caller's case, redundant ad wasted. Since allocation paths must be made as efficient as possible, we should (in the future) try and get these to be more efficient and avoid such redundancy where possible.
Some of these checks are still required when ElasticTLABs is disabled.
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 275:
>
>> 273: assert(result != nullptr, "Allocation cannot fail");
>> 274: assert(r->top() <= r->end(), "Allocation cannot span end of region");
>> 275: assert(req.actual_size() % CardTable::card_size_in_words() == 0, "PLAB size must be multiple of card size");
>
> We've already checked at line 270 that `size % CardTable::card_size_in_words() == 0`. I think we can check a stronger condition here, i.e.:
>
> assert(req.actual_size() == size, "Unexpected actual_size()");
Hmm... I'm not sure about that. In the case when elastic TLABs is enabled, we will have already decreased size to fit in the region and so we don't expect to `allocate_aligned` to further decrease the size. However, in the case when elastic TLABs is disabled we can't be sure that `allocate_aligned` will not set `actual_size` to something smaller than the requested `size`. This arguably violates the intention behind disabling elastic TLABs, but I think this is a matter for a different PR?
-------------
PR Review Comment: https://git.openjdk.org/shenandoah/pull/260#discussion_r1170679200
PR Review Comment: https://git.openjdk.org/shenandoah/pull/260#discussion_r1170678883
More information about the shenandoah-dev
mailing list