RFR: 8253779: Amalloc may be wasting space by overaligning [v2]

Kim Barrett kbarrett at openjdk.java.net
Fri Jul 9 04:07:53 UTC 2021


On Fri, 9 Jul 2021 00:36:17 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> 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 ?  Is there now a preference?

`static_assert` is new in C++11, `STATIC_ASSERT` is a macro that provides somewhat similar functionality (but without the informative message) that works before C++11. I think we should prefer `static_assert` now; the only (arguable) downside is that (until C++17) the informative message is required.

>> 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.

Perhaps the naming ought to be Amalloc => AmallocL (long) and Amalloc4 => Amalloc?  But even if you agree with that or some other better naming scheme, such renaming probably ought to be a separate thing.

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

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


More information about the hotspot-dev mailing list