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

[Thomas Stuefe]

On Fri, 7 Jun 2024 10:49:54 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> Now we use access specifiers to deny access to the index, except for the allocator, which is a friend. We do want the index to be opaque, so we do this. On top of that, we use an `_owner` field in debug mode to assert that we don't free to the wrong allocator.

The last part I don't like. It causes `I` to quadruple in size in debug builds only. For something that is potentially used as member in bunch of different data structures, it can cause considerable deltas in memory layout between debug and release builds. I don't think that is a good thing. What you test in debug should be close to what's running at a customer.

I also don't think it is particularly useful. I would instead do a simple range check on I. That gives you a part of the sanity checks without the negatives. Couple that with a sanity check wrt to slot state (don't free free slots) you are pretty well covered already.

Other than that, I think using an own class to hide a simple i32 is unnecessary complex, but I leave that up to you. If you do this, please take care of constness. And make it work for its money, e.g. by giving it an is_nil instead of exposing nil, and requiring the user to manually compare it.

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

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