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

[Thomas Stuefe]

On Wed, 12 Jun 2024 15:20:51 GMT, Johan Sjölen <jsjolen at openjdk.org> wrote:

> I need to take a think about this, it's quite annoying that we're generating a new `I` per `IFLA<E, flag>`. I think that we could make it a global struct `IndexPointer`, or something like that.

TBH, I am quite opinionated about this. One example:

Yesterday, I tried to rework the VMATreeTest.TestConsistencyWithSimpleTracker_vm. See issue https://bugs.openjdk.org/browse/JDK-8334179. I first tried to implement a more efficient memory layout for the SimpleVMATracker (everything in SimpleVMATracker::Info could, essentially, stored in a single byte, giving us a much better cache performance). But for that, I would have needed the numerical value of the stack index. But that is hidden away in NativeCallStackStorage::StackIndex. Apart from giving me RSI from typing, I cannot just extract the integer from NativeCallStackStorage::StackIndex since its sole purpose in life is to hide that integer. Of course, I can declare yet another friend class. But that's ugly and does not scale.

I really abhor the friend class concept since it subverts good encapsulation. It means I have different classes that can freely rummage around in my innards. So I need IDE introspection to actually know whats going on. For a terrible example of how complex this gets see Chunk and Arena and all its child classes, and try to figure out who does modify the Arena innards when and why. Compare that with Metaspace, which is the spiritual successor of Arenas. Much better separation of concerns and encapsulation.

I can kind of see the point with test classes, but even then, a better way would be to make the test class an inner class.

TL;DR: I think these wrappers make life difficult while giving only minuscule protection against errors - if any at all.

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

PR Comment: https://git.openjdk.org/jdk/pull/18979#issuecomment-2164955577