On Wed, 3 Jun 2026 12:57:56 GMT, Per Minborg <[email protected]> wrote:
>> My feeling is the same here. Looking at the patch it feels like the control >> is inverted. I'd expect either ArenaImpl to be changed directly to do what >> Maurizio lists here, or to have a `ConfinedArenaImpl` sub class that does >> these things (but I think from internal discussion we wanted to do something >> that works for shared/auto as well) >> >> In my mind we don't want a different arena impl, we want the existing arena >> impls do to something different, if that makes sense. > >> More specifically, I think that in both the confined and shared/auto case, >> what you want is a setup like this: >> >> 1. when an arena is created, we try to acquire some memory from some pool >> 2. if we fail, then the arena behaves like before >> 3. otherwise, the first N allocations of the arena will be served by the pool >> 4. after that, we either try to acquire another pool, or fallback to default >> impl >> >> The only difference between confined and shared is where the pool lives, and >> how it is acquired. > > There are some issues with the proposal: > 1. If there are no allocations or if there are allocations that are _larger_ > than the pool size, we cannot defer allocation from the pool. I think we can > adhere to the principle outlined above, but do lazy allocation instead. > 4. We then need to be able to handle N slots per arena. That is doable but > more complex. I think that just increasing the pool size would achieve the > same thing (or would be even better), but with lower complexity. I.e., > setting the pool size to 128 bytes rather than having two separate 64-byte > pools. > > The work with other arenas is future work, but if we can have a better > structure that allows them to be more easily added in the future, that is > good. > My general feeling here is that the implementation is arranged the wrong way. > E.g. in my mind, we have ArenaImpl, which is the type of the builtin arena we > return. And, if an ArenaImpl is confined, it can allocate memory more > cheaply, with the help of some kind of thread-backed allocator. > > I feel the right arrangement is to have a SegmentAllocator (not an Arena) > that returns usable regions of memory from a given thread. Maybe the > allocator is very low level, it computes the next pointer, and does a > `MemorySegment.ofAddress(ptr)` for the region. Then the ArenaImpl::allocate > takes that, and does a reinterpret with the correct arena and size. When the > confined arena closes, the memory is returned to the underlying pool. > > Since this is the builtin confined arena we're talking about, I'm not sure > about CachedArena -- as that looks like any other 3rd party Arena. I think we > can achieve tighter integration? There are some good suggestions here worth exploring. In one of the prototypes, I extended `ArenaImpl`, but eventually I opted for a more decoupled approach. But I can take a new look at centralizing logic in `ArenaImpl` instead. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31365#discussion_r3348668305
