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

[Johan Sjölen]

On Sat, 15 Jun 2024 06:45:26 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:
>> 
>>   Remove dead CHeap allocator test
>
> src/hotspot/share/nmt/indexedFreeListAllocator.hpp line 79:
> 
>> 77: 
>> 78:   void free(I i) {
>> 79:     assert(i != nil || (i > 0 && i < _backing_storage.length()), "out of bounds free");
> 
> I think there are some errors here. This is probably broken. Which we would see if the gtests were running, but hotspot common tier1 tests seem broken.
> 
> Do we allow passing in nil? Then, i must be either nil or valid, not != nil or valid. If not, use an AND, not an OR.
> i=0 is valid
> Could you also please factor out OOB test for i?

It's meant to express `P => Q` equiv. to `~P V Q`, but we're trying to say "is_not_nil(i) => is_valid(i)`, so it should be `i == nil` **not** `i != nil`. Yes, it's broken. Also yes, let's make OOB check a predicate function.

> src/hotspot/share/nmt/indexedFreeListAllocator.hpp line 80:
> 
>> 78:   void free(I i) {
>> 79:     assert(i != nil || (i > 0 && i < _backing_storage.length()), "out of bounds free");
>> 80:     if (i != nil) return;
> 
> i == nil?

Not sure how I managed this miracle of coding :), yes.

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

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