RFR: 8253779: Amalloc may be wasting space by overaligning

Coleen Phillimore coleenp at openjdk.java.net
Fri Jul 9 00:38:50 UTC 2021


On Thu, 8 Jul 2021 22:33:29 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Thanks to @kimbarrett for noticing this. The alignment was changed to 64 bits for 32 bit platforms, but overalign for 64 bits platforms.  I changed this to BytesPerLong to cover both, since the long case is why it was changed on 32 bits in the first place in JDK-4526490.
>> I also removed Amalloc_D since I don't know what D stands for and it's the same as Amalloc_4.  That's not a great name either. I'm open to suggestions!
>> Tested with tier1-3.
>
> src/hotspot/share/memory/arena.hpp line 140:
> 
>> 138:   // on both 32 and 64 bit platforms. Required for atomic long operations on 32 bits.
>> 139:   void* Amalloc(size_t x, AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
>> 140:     assert(is_power_of_2(ARENA_AMALLOC_ALIGNMENT) , "should be a power of 2");
> 
> [pre-existing] I think this could now be a static_assert.

Changing it so.  Why is there capital STATIC_ASSERT and not capital static_assert ?

> src/hotspot/share/memory/arena.hpp line 155:
> 
>> 153: 
>> 154:   // Allocate in the arena, assuming the size has been aligned to size of pointer, which
>> 155:   // is 4 bytes on 32 bits, hence the name.
> 
> `Amalloc_4` is a horrible name for this, because this is allocating pointer-aligned and sized values, and even enforces that the size is appropriately pointer aligned.  Maybe better would be something like `AmallocP`?  Though it can also be used to allocate `size_t` and the like that are non-pointers.

I agree that Amalloc_4 is a bad name.  I thought of Amalloc_ptr but it's not good either.  Amalloc_naturally_aligned....  too long.

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

PR: https://git.openjdk.java.net/jdk/pull/4732


More information about the hotspot-dev mailing list