RFR: 8332139: SymbolTableHash::Node allocations allocates twice the required memory

[Axel Boldt-Christmas]

On Tue, 4 Jun 2024 15:40:41 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/classfile/symbolTable.cpp line 185:
>> 
>>> 183: private:
>>> 184:   static void* allocate_node_impl(size_t size, Value const& value) {
>>> 185:     size_t alloc_size = SymbolTableHash::get_dynamic_node_size(value.byte_size());
>> 
>> So 'size' passed in is actually sizeof(NODE), right or is it sizeof(NODE) + sizeof(VALUE) ?  So also to fix it, don't we just remove value.byte_size() from this calculation?
>
> The real problem is that size passed in is known to the concurrent hash table but not to the caller, so maybe this does help make this less error prone.

> So 'size' passed in is actually sizeof(NODE), right or is it sizeof(NODE) + sizeof(VALUE) ?

It is `sizeof(NODE)`. Which for types that does not have exotic alignment would end up being `sizeof(void*) /* node metadata */ + sizeof(VALUE) /* payload/data */`

> So also to fix it, don't we just remove value.byte_size() from this calculation?

That is correct, but it does require implementation details of CHT to leak all the way into SymbolTableConfig.

_There is an implementation of CHT that does not care about the storage requirements of the VALUE, nor needs to know how to create it. One that only works with `void*` and off loads not only the allocation, but also the construction and destruction to the Config. (Would also require more care with alignment)_

The attempt with this patch is to create an abstraction where the Config does not need to know about the implementation details of the CHT node. So now:
* `static void* allocate_node(void* context, size_t size, Value const& value)` allows the CHT to query the Config for an allocation of at least `size` into which the value object will be copy constructed.
  * If the `Value` is dynamically sized (has extended storage beyond the end of the C++ object) the Config can query the CHT for an updated `size` given the `value`'s dynamic size requirements.
    * This also enforces that the Value's type and object layout is compatible with the CHT's Node implementation.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19214#discussion_r1627231890