RFR: 8333658: NMT: Use an allocator with 4-byte pointers to save memory in NativeCallStackStorage [v21]

[Johan Sjölen]

On Tue, 18 Jun 2024 06:16:30 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix tests
>
> src/hotspot/share/nmt/indexedFreeListAllocator.hpp line 38:
> 
>> 36: class IndexedFreeListAllocator {
>> 37: public:
>> 38:   using I = int32_t;
> 
> Proposal, possibly for a follow-up RFE:
> 
> - its a pity to bury this in nmt, we may have uses for it elsewhere
> - If we only have a small number of elements, I don't want to burn 4 bytes on an index.
> - I dislike the fact that we burn half the value range of index to "invalid"
> - AFAICS we don't have a index overflow check in allocate (what happens with >2 billion elements?)
> 
> So:
> 
> 
> - make this a general-purpose container in utilities
> - I would make the index type a template parameter, (X) with the rule that it should be an unsigned integral (and STATIC_ASSERT that)
> - I would then:
> 
> 
> constexpr X nil = std::numeric_limits<X>::max();
> constexpr X max = nil - 1;
> 
> 
> Then, down in allocate, make sure we don't go over `max`.
> 
> Now, If I know that I only need e.g. <255 elements, I can use a single byte as index, that could give me good packing depending on where the index is stored. And we get index overflow checking inbuilt, too.

This is what I'd like to have in a follow-up RFE. Your last two points are because of `GrowableArray`: GA uses int32, so we do, and GA checks overflow when expanding, so we're OK.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18979#discussion_r1647196145