RFR: 8329134: Reconsider TLAB zapping

[Stefan Karlsson]

On Wed, 27 Mar 2024 13:42:08 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> We zap the entire TLAB on initial allocation (`MemAllocator::mem_allocate_inside_tlab_slow`), and then also rezap the object contents when object is allocated from the TLAB (`ThreadLocalAllocBuffer::allocate`). The second part seems excessive, given the TLAB is already fully zapped.
>> 
>> There is also no way to disable this zapping, like you would in other places with the relevant Zap* flags.
>> 
>> Fixing both these issues allows to improve fastdebug tests performance, e.g. in jcstress. 
>> 
>> It also allows to remove the related Zero kludge.
>> 
>> Additional testing:
>>  - [ ] Linux AArch64 server fastdebug, `all` tests 
>>  - [x] MacOS AArch64 Zero fastdebug, `bootcycle-images` pass
>
> 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.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18500#discussion_r1541177284