RFR: Usage tracking cleanup
William Kemper
wkemper at openjdk.org
Tue Apr 18 22:50:20 UTC 2023
On Tue, 18 Apr 2023 22:03:43 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:
>> I have left a few comments. Despite the improvements that you have made by some of the current refactoring, I am concerned about the organization of the allocation paths and believe that there are opportunities for cleaning these up, and specializing and slimming these allocation paths a bit more.
>>
>> That can wait for later, but I'd like to take another pass at this once you have had a chance to read the review comments and in case those lead to any changes.
>>
>> Thanks!
>
>> I have left a few comments. Despite the improvements that you have made by some of the current refactoring, I am concerned about the organization of the allocation paths and believe that there are opportunities for cleaning these up, and specializing and slimming these allocation paths a bit more.
>>
>> That can wait for later, but I'd like to take another pass at this once you have had a chance to read the review comments and in case those lead to any changes.
>>
>> Thanks!
>
> This is a good clean up, now that I see the big picture of the consolidation of the adjustments. I'll approve once you have had a chance to consider my review comments.
Thanks for the review @ysramakrishna . I'll make changes based on your suggestions.
>> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 358:
>>
>>> 356: size_t usable_free = get_usable_free_words(r->free());
>>> 357: if (size <= usable_free) {
>>> 358: result = allocated_aligned_plab(size, req, r);
>>
>> `allocate_..`
>
> Isn't this likely to fail -- at least in the general case -- the alignment checks in assertions in `allocate_aligned_plab()`?
>
> I am not sure I understand the difference between `ShenandoahElasticTLAB` and not, since the documentation comment isn't very helpful:
>
> product(bool, ShenandoahElasticTLAB, true, DIAGNOSTIC, \
> "Use Elastic TLABs with Shenandoah") \
When `ShenandoahElasticTLAB` is enabled, the allocator is allowed to return a TLAB smaller than the requested size (but greater than the `min_size`).
-------------
PR Comment: https://git.openjdk.org/shenandoah/pull/260#issuecomment-1513879200
PR Review Comment: https://git.openjdk.org/shenandoah/pull/260#discussion_r1170633707
More information about the shenandoah-dev
mailing list