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