RFR: 8329134: Reconsider TLAB zapping [v2]
Aleksey Shipilev
shade at openjdk.org
Wed Mar 27 14:55:35 UTC 2024
On Wed, 27 Mar 2024 14:00:56 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> src/hotspot/share/gc/shared/memAllocator.cpp line 323:
>>
>>> 321: Copy::zero_to_words(mem, allocation._allocated_tlab_size);
>>> 322: } else if (ZapTLAB) {
>>> 323: // ...and zap just allocated TLAB.
>>
>> Could you add a blank line after this, so that the two separate comments don't get bunched together?
>
> Rereading the patch, I also so that we have:
>
> // ..and clear it.
> ...
> // ...and zap just allocated TLAB.
>
> Maybe just unify this into:
>
> // ..and clear it.
> ...
> // ...and zap it.
>
> Alternatively:
>
> // ..and clear just allocated TLAB.
> ...
> // ...and zap just allocated TLAB.
>
>
> And while you are changing this. What's up with the `..` vs `...`? (rhetoric question) I'd prefer if we just picked either of those.
>
> This comment also applies to the duplicated code in shenandoahHeap.cpp.
I meditated about this code a bit, and I think we have a cleaner third option, see new commit.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18500#discussion_r1541280635
More information about the shenandoah-dev
mailing list