On Wed, 22 Jan 2025 10:05:31 GMT, Matthias Ernst <d...@openjdk.org> wrote:
>> Matthias Ernst has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Back buffer allocation with a single carrier-local segment.
>
> src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java line
> 389:
>
>> 387: @Override
>> 388: protected BufferStack initialValue() {
>> 389: return new BufferStack(Arena.ofAuto().allocate(256));
>
> Could as well use keep using TerminatingThreadLocal+Unsafe here, I just like
> the fact to use as few non-public apis as possible.
I'm told that TerminatingThreadLocal runs the "terminate" action for an object
T from the same thread T refers to. So, in principle, using a
TerminatingThreadLocal + confined arena should be ok.
If that works, I'd suggest to consider maybe moving all this sharing logic
inside BufferStack, so that users only need to do:
static final BufferStack LINKER_BUFFER = new BufferStack(256);
...
try (Arena arena = BufferStack.reserve(size)) {
...
}
Which seems a more re-usable API. After all, the fact that we decide to lock or
not in certain cases is heavily influenced by the fact that we expect a
BufferStack to be used with a carrier local, so we might just as well fold the
caching in there.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23142#discussion_r1925113881
