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