From b309aac2197e1353a9bacb10830e5b0f6b89881f Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Fri, 7 Feb 2025 13:01:37 +0000 Subject: [PATCH 01/12] Add method handle adapter for system calls --- .../java/lang/foreign/SegmentAllocator.java | 13 +- .../jdk/internal/foreign/ArenaImpl.java | 11 +- .../internal/foreign/CaptureStateUtil.java | 288 +++++++++++++++++ .../foreign/CarrierLocalArenaPools.java | 282 +++++++++++++++++ .../foreign/NoInitSegmentAllocator.java | 43 +++ .../classes/jdk/internal/invoke/MhUtil.java | 10 + .../java/foreign/TestCaptureStateUtil.java | 150 +++++++++ .../foreign/TestCarrierLocalArenaPools.java | 295 ++++++++++++++++++ .../lang/foreign/CaptureStateUtilBench.java | 130 ++++++++ 9 files changed, 1212 insertions(+), 10 deletions(-) create mode 100644 src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java create mode 100644 src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java create mode 100644 src/java.base/share/classes/jdk/internal/foreign/NoInitSegmentAllocator.java create mode 100644 test/jdk/java/foreign/TestCaptureStateUtil.java create mode 100644 test/jdk/java/foreign/TestCarrierLocalArenaPools.java create mode 100644 test/micro/org/openjdk/bench/java/lang/foreign/CaptureStateUtilBench.java diff --git a/src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java b/src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java index 1297406dcf194..56d38834f2b78 100644 --- a/src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java +++ b/src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java @@ -27,6 +27,7 @@ import jdk.internal.foreign.AbstractMemorySegmentImpl; import jdk.internal.foreign.ArenaImpl; +import jdk.internal.foreign.NoInitSegmentAllocator; import jdk.internal.foreign.SlicingAllocator; import jdk.internal.foreign.StringSupport; import jdk.internal.vm.annotation.ForceInline; @@ -720,22 +721,22 @@ private static void assertWritable(MemorySegment segment) { @ForceInline private MemorySegment allocateNoInit(long byteSize) { - return this instanceof ArenaImpl arenaImpl ? - arenaImpl.allocateNoInit(byteSize, 1) : + return this instanceof NoInitSegmentAllocator noInit ? + noInit.allocateNoInit(byteSize, 1) : allocate(byteSize); } @ForceInline private MemorySegment allocateNoInit(MemoryLayout layout) { - return this instanceof ArenaImpl arenaImpl ? - arenaImpl.allocateNoInit(layout.byteSize(), layout.byteAlignment()) : + return this instanceof NoInitSegmentAllocator noInit ? + noInit.allocateNoInit(layout.byteSize(), layout.byteAlignment()) : allocate(layout); } @ForceInline private MemorySegment allocateNoInit(MemoryLayout layout, long size) { - return this instanceof ArenaImpl arenaImpl ? - arenaImpl.allocateNoInit(layout.byteSize() * size, layout.byteAlignment()) : + return this instanceof NoInitSegmentAllocator noInit ? + noInit.allocateNoInit(layout.byteSize() * size, layout.byteAlignment()) : allocate(layout, size); } } diff --git a/src/java.base/share/classes/jdk/internal/foreign/ArenaImpl.java b/src/java.base/share/classes/jdk/internal/foreign/ArenaImpl.java index da8a6c2666637..a9df67c6c4ff2 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/ArenaImpl.java +++ b/src/java.base/share/classes/jdk/internal/foreign/ArenaImpl.java @@ -25,10 +25,12 @@ package jdk.internal.foreign; +import jdk.internal.vm.annotation.ForceInline; + import java.lang.foreign.Arena; import java.lang.foreign.MemorySegment.Scope; -public final class ArenaImpl implements Arena { +public final class ArenaImpl implements Arena, NoInitSegmentAllocator { private final MemorySessionImpl session; private final boolean shouldReserveMemory; @@ -47,15 +49,16 @@ public void close() { session.close(); } + @ForceInline + @Override public NativeMemorySegmentImpl allocateNoInit(long byteSize, long byteAlignment) { Utils.checkAllocationSizeAndAlign(byteSize, byteAlignment); return SegmentFactories.allocateSegment(byteSize, byteAlignment, session, shouldReserveMemory); } + @ForceInline @Override public NativeMemorySegmentImpl allocate(long byteSize, long byteAlignment) { - NativeMemorySegmentImpl segment = allocateNoInit(byteSize, byteAlignment); - segment.fill((byte)0); - return segment; + return NoInitSegmentAllocator.super.allocate(byteSize, byteAlignment); } } diff --git a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java new file mode 100644 index 0000000000000..da7efd381b43f --- /dev/null +++ b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java @@ -0,0 +1,288 @@ +/* + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.internal.foreign; + +import jdk.internal.invoke.MhUtil; +import jdk.internal.vm.annotation.ForceInline; + +import java.lang.foreign.Arena; +import java.lang.foreign.Linker; +import java.lang.foreign.MemoryLayout; +import java.lang.foreign.MemorySegment; +import java.lang.foreign.StructLayout; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.lang.invoke.VarHandle; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; + +public final class CaptureStateUtil { + + private static final MemoryLayout CAPTURE_LAYOUT = Linker.Option.captureStateLayout(); + //private static final CarrierLocalArenaPools POOL = CarrierLocalArenaPools.create(CAPTURE_LAYOUT); + private static final CarrierLocalArenaPools POOL = CarrierLocalArenaPools.create(16); + + private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); + + private static final MethodHandle NON_NEGATIVE_INT_MH = + MhUtil.findStatic(LOOKUP, "nonNegative", + MethodType.methodType(boolean.class, int.class)); + + private static final MethodHandle SUCCESS_INT_MH = + MhUtil.findStatic(LOOKUP, "success", + MethodType.methodType(int.class, int.class, MemorySegment.class)); + + private static final MethodHandle ERROR_INT_MH = + MhUtil.findStatic(LOOKUP, "error", + MethodType.methodType(int.class, MethodHandle.class, int.class, MemorySegment.class)); + + private static final MethodHandle NON_NEGATIVE_LONG_MH = + MhUtil.findStatic(LOOKUP, "nonNegative", + MethodType.methodType(boolean.class, long.class)); + + private static final MethodHandle SUCCESS_LONG_MH = + MhUtil.findStatic(LOOKUP, "success", + MethodType.methodType(long.class, long.class, MemorySegment.class)); + + private static final MethodHandle ERROR_LONG_MH = + MhUtil.findStatic(LOOKUP, "error", + MethodType.methodType(long.class, MethodHandle.class, long.class, MemorySegment.class)); + + private static final MethodHandle ACQUIRE_ARENA_MH = + MhUtil.findStatic(LOOKUP, "acquireArena", + MethodType.methodType(Arena.class)); + + private static final MethodHandle ALLOCATE_MH = + MhUtil.findStatic(LOOKUP, "allocate", + MethodType.methodType(MemorySegment.class, Arena.class)); + + private static final MethodHandle ARENA_CLOSE_MH = + MhUtil.findVirtual(LOOKUP, Arena.class, "close", + MethodType.methodType(void.class)); + + // (int.class | long.class) -> + // ({"GetLastError" | "WSAGetLastError"} | "errno") -> + // MethodHandle + private static final Map, Map> INNER_HANDLES; + + static { + + final StructLayout stateLayout = Linker.Option.captureStateLayout(); + final Map, Map> classMap = new HashMap<>(); + for (var returnType : new Class[]{int.class, long.class}) { + Map handles = stateLayout + .memberLayouts().stream() + .collect(Collectors.toUnmodifiableMap( + member -> member.name().orElseThrow(), + member -> { + VarHandle vh = stateLayout.varHandle(MemoryLayout.PathElement.groupElement(member.name().orElseThrow())); + // (MemorySegment, long)int + MethodHandle intExtractor = vh.toMethodHandle(VarHandle.AccessMode.GET); + // (MemorySegment)int + intExtractor = MethodHandles.insertArguments(intExtractor, 1, 0L); + + if (returnType.equals(int.class)) { + // (int, MemorySegment)int + return MethodHandles.guardWithTest( + NON_NEGATIVE_INT_MH, + SUCCESS_INT_MH, + ERROR_INT_MH.bindTo(intExtractor)); + } else { + // (long, MemorySegment)long + return MethodHandles.guardWithTest( + NON_NEGATIVE_LONG_MH, + SUCCESS_LONG_MH, + ERROR_LONG_MH.bindTo(intExtractor)); + } + } + )); + classMap.put(returnType, handles); + } + INNER_HANDLES = Map.copyOf(classMap); + } + + private CaptureStateUtil() { + } + + /** + * {@return a new MethodHandle that adapts the provided {@code target} so that it + * directly returns the same value as the {@code target} if it is + * non-negative, otherwise returns the negated errno} + *

+ * This method is suitable for adapting system-call method handles(e.g. + * {@code open()}, {@code read()}, and {@code close()}). Clients can check the return + * value as shown in this example: + * {@snippet lang = java: + * // (MemorySegment capture, MemorySegment pathname, int flags)int + * static final MethodHandle CAPTURING_OPEN = ... + * + * // (MemorySegment pathname, int flags)int + * static final MethodHandle OPEN = CaptureStateUtil.adaptSystemCall(Pooling.GLOBAL, CAPTURING_OPEN, "errno"); + * + * try { + * int fh = (int)OPEN.invoke(pathName, flags); + * if (fh < 0) { + * throw new IOException("Error opening file: errno = " + (-fh)); + * } + * processFile(fh); + * } catch (Throwable t) { + * throw new RuntimeException(t); + * } + * + *} + * For a method handle that takes a MemorySegment and two int parameters using GLOBAL, + * the method combinators are doing the equivalent of: + * + * {@snippet lang = java: + * public int invoke(int a, int b) { + * final MemorySegment segment = acquireSegment(); + * final int result = (int) handle.invoke(segment, a, b); + * if (result >= 0) { + * return result; + * } + * return -(int) errorHandle.get(segment); + * } + *} + * Where {@code handle} is the original method handle with the coordinated + * {@code (MemorySegment, int, int)int} and {@code errnoHandle} is a method handle + * that retrieves the error code from the capturing segment. + * + * + * @param target method handle that returns an {@code int} or a {@code long} and has + * a capturing state MemorySegment as its first parameter + * @param stateName the name of the capturing state member layout + * (i.e. "errno","GetLastError", or "WSAGetLastError") + * @throws IllegalArgumentException if the provided {@code target}'s return type is + * not {@code int} or {@code long} + * @throws IllegalArgumentException if the provided {@code target}'s first parameter + * type is not {@linkplain MemorySegment} + * @throws IllegalArgumentException if the provided {@code stateName} is unknown + * on the current platform + */ + public static MethodHandle adaptSystemCall(MethodHandle target, + String stateName) { + // Implicit null check + final Class returnType = target.type().returnType(); + Objects.requireNonNull(stateName); + + if (!(returnType.equals(int.class) || returnType.equals(long.class))) { + throw illegalArgDoesNot(target, "return an int or a long"); + } + if (target.type().parameterCount() == 0 || target.type().parameterType(0) != MemorySegment.class) { + throw illegalArgDoesNot(target, "have a MemorySegment as the first parameter"); + } + + // ((int | long), MemorySegment)(int | long) + MethodHandle inner = INNER_HANDLES + .get(returnType) + .get(stateName); + if (inner == null) { + throw new IllegalArgumentException("Unknown state name: " + stateName + + ". Known on this platform: " + Linker.Option.captureStateLayout()); + } + + // Make `target` specific adaptations + + // (C0=MemorySegment, C1-Cn, MemorySegment)(int|long) + inner = MethodHandles.collectArguments(inner, 0, target); + + int[] perm = new int[target.type().parameterCount() + 1]; + for (int i = 0; i < target.type().parameterCount(); i++) { + perm[i] = i; + } + perm[target.type().parameterCount()] = 0; + // Deduplicate the first and last coordinate and only use the first + // (C0=MemorySegment, C1-Cn)(int|long) + inner = MethodHandles.permuteArguments(inner, target.type(), perm); + // (C0=Arena, C1-Cn)(int|long) + inner = MethodHandles.collectArguments(inner, 0, ALLOCATE_MH); + + // ((int|long))(int|long) + MethodHandle cleanup = MethodHandles.identity(returnType); + // (Throwable, (int|long))(int|long) + cleanup = MethodHandles.dropArguments(cleanup, 0, Throwable.class); + // (Throwable, (int|long), Arena)(int|long) + // Cleanup does not have to have all parameters. It can have zero or more. + cleanup = MethodHandles.collectArguments(cleanup, 2, ARENA_CLOSE_MH); + + // (Arena, C1-Cn)(int|long) + MethodHandle tryFinally = MethodHandles.tryFinally(inner, cleanup); + + // Finally we arrive at (C1-Cn)(int|long) + return MethodHandles.collectArguments(tryFinally, 0, ACQUIRE_ARENA_MH); + } + + private static IllegalArgumentException illegalArgDoesNot(MethodHandle target, String info) { + return new IllegalArgumentException("The provided target " + target + + " does not " + info); + } + + // The methods below are reflective used via static MethodHandles + + @ForceInline + private static Arena acquireArena() { + return POOL.take(); + } + + @ForceInline + private static MemorySegment allocate(Arena arena) { + // We do not need to zero out the segment. + return ((NoInitSegmentAllocator) arena) + .allocateNoInit(CAPTURE_LAYOUT.byteSize(), CAPTURE_LAYOUT.byteAlignment()); + } + + @ForceInline + private static boolean nonNegative(int value) { + return value >= 0; + } + + @ForceInline + private static int success(int value, MemorySegment segment) { + return value; + } + + @ForceInline + private static int error(MethodHandle errorHandle, int value, MemorySegment segment) throws Throwable { + return -(int) errorHandle.invokeExact(segment); + } + + @ForceInline + private static boolean nonNegative(long value) { + return value >= 0L; + } + + @ForceInline + private static long success(long value, MemorySegment segment) { + return value; + } + + @ForceInline + private static long error(MethodHandle errorHandle, long value, MemorySegment segment) throws Throwable { + return -(int) errorHandle.invokeExact(segment); + } + +} diff --git a/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java b/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java new file mode 100644 index 0000000000000..ddfe6199d2184 --- /dev/null +++ b/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java @@ -0,0 +1,282 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.internal.foreign; + +import jdk.internal.access.JavaLangAccess; +import jdk.internal.access.SharedSecrets; +import jdk.internal.misc.CarrierThread; +import jdk.internal.misc.TerminatingThreadLocal; +import jdk.internal.misc.Unsafe; +import jdk.internal.vm.annotation.ForceInline; +import jdk.internal.vm.annotation.Stable; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemoryLayout; +import java.lang.foreign.MemorySegment; +import java.util.Objects; + +public final class CarrierLocalArenaPools { + + @Stable + private final TerminatingThreadLocal tl; + + private CarrierLocalArenaPools(long byteSize, long byteAlignment) { + this.tl = new TerminatingThreadLocal<>() { + + private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess(); + + @Override + protected LocalArenaPoolImpl initialValue() { + if (JLA.currentCarrierThread() instanceof CarrierThread) { + // Only a carrier thread that is an instance of `CarrierThread` can + // ever carry virtual threads. (Notably, a `CarrierThread` can also + // carry a platform thread.) This means a `CarrierThread` can carry + // any number of virtual threads, and they can be mounted/unmounted + // from the carrier thread at almost any time. Therefore, we must use + // stronger-than-plain semantics when dealing with mutual exclusion + // of thread local resources. + return new LocalArenaPoolImpl.OfCarrier(byteSize, byteAlignment); + } else { + // A carrier thread that is not an instance of `CarrierThread` can + // never carry a virtual thread. Because of this, only one thread will + // be mounted on such a carrier thread. Therefore, we can use plain + // memory semantics when dealing with mutual exclusion of thread local + // resources. + return new LocalArenaPoolImpl.OfPlatform(byteSize, byteAlignment); + } + } + + @Override + protected void threadTerminated(LocalArenaPoolImpl stack) { + stack.close(); + } + }; + } + + @ForceInline + public Arena take() { + return tl.get() + .take(); + } + + private static sealed abstract class LocalArenaPoolImpl { + + static final int AVAILABLE = 0; + static final int TAKEN = 1; + + @Stable + private final Arena pooledArena; + @Stable + private final MemorySegment segment; + // Used both directly and reflectively + int segmentAvailability; + + private LocalArenaPoolImpl(long byteSize, + long byteAlignment) { + this.pooledArena = Arena.ofConfined(); + this.segment = pooledArena.allocate(byteSize, byteAlignment); + } + + @ForceInline + public final Arena take() { + final Arena arena = Arena.ofConfined(); + return tryAcquireSegment() + ? new SlicingArena((ArenaImpl) arena, segment) + : arena; + } + + public final void close() { + pooledArena.close(); + } + + /** + * {@return {@code true } if the segment was acquired for exclusive use, {@code + * false} otherwise} + */ + abstract boolean tryAcquireSegment(); + + /** + * Unconditionally releases the acquired segment if it was previously acquired, + * otherwise this is a no-op. + */ + abstract void releaseSegment(); + + /** + * Thread safe implementation. + */ + public static final class OfCarrier + extends LocalArenaPoolImpl { + + // Unsafe allows earlier use in the init sequence and + // better start and warmup properties. + static final Unsafe UNSAFE = Unsafe.getUnsafe(); + static final long SEG_AVAIL_OFFSET = + UNSAFE.objectFieldOffset(LocalArenaPoolImpl.class, "segmentAvailability"); + + public OfCarrier(long byteSize, + long byteAlignment) { + super(byteSize, byteAlignment); + } + + @ForceInline + boolean tryAcquireSegment() { + return UNSAFE.compareAndSetInt(this, SEG_AVAIL_OFFSET, AVAILABLE, TAKEN); + } + + @ForceInline + void releaseSegment() { + UNSAFE.putIntVolatile(this, SEG_AVAIL_OFFSET, AVAILABLE); + } + } + + /** + * No need for thread-safe implementation here as a platform thread is exclusively + * mounted on a particular carrier thread. + */ + public static final class OfPlatform + extends LocalArenaPoolImpl { + + public OfPlatform(long byteSize, + long byteAlignment) { + super(byteSize, byteAlignment); + } + + @ForceInline + boolean tryAcquireSegment() { + if (segmentAvailability == TAKEN) { + return false; + } else { + segmentAvailability = TAKEN; + return true; + } + } + + @ForceInline + void releaseSegment() { + segmentAvailability = AVAILABLE; + } + } + + /** + * A SlicingArena is similar to a {@linkplain SlicingAllocator} but if the backing + * segment cannot be used for allocation, a fall-back arena is used instead. This + * means allocation never fails due to the size and alignment of the backing + * segment. + *

+ * Todo: Should we expose a variant of this class as a complement + * to SlicingAllocator? + */ + private final class SlicingArena implements Arena, NoInitSegmentAllocator { + + @Stable + private final ArenaImpl delegate; + @Stable + private final MemorySegment segment; + + private long sp = 0L; + + @ForceInline + private SlicingArena(ArenaImpl arena, + MemorySegment segment) { + this.delegate = arena; + this.segment = segment; + } + + @ForceInline + @Override + public MemorySegment.Scope scope() { + return delegate.scope(); + } + + @ForceInline + @Override + public NativeMemorySegmentImpl allocate(long byteSize, long byteAlignment) { + return NoInitSegmentAllocator.super.allocate(byteSize, byteAlignment); + } + + @SuppressWarnings("restricted") + @ForceInline + public NativeMemorySegmentImpl allocateNoInit(long byteSize, long byteAlignment) { + final long min = segment.address(); + final long start = Utils.alignUp(min + sp, byteAlignment) - min; + if (start + byteSize <= segment.byteSize()) { + Utils.checkAllocationSizeAndAlign(byteSize, byteAlignment); + final MemorySegment slice = segment.asSlice(start, byteSize, byteAlignment); + sp = start + byteSize; + return fastReinterpret(delegate, (NativeMemorySegmentImpl) slice, byteSize); + } else { + return delegate.allocateNoInit(byteSize, byteAlignment); + } + } + + @ForceInline + @Override + public void close() { + delegate.close(); + // Intentionally do not releaseSegment() in a finally clause as + // the segment still is in play if close() initially fails (e.g. is closed + // from a non-owner thread). Later on the close() method might be + // successfully re-invoked (e.g. from its owner thread). + LocalArenaPoolImpl.this.releaseSegment(); + } + } + } + + // Equivalent to: + // return (NativeMemorySegmentImpl) slice + // .reinterpret(byteSize, delegate, null); */ + @ForceInline + static NativeMemorySegmentImpl fastReinterpret(ArenaImpl arena, + NativeMemorySegmentImpl segment, + long byteSize) { + // We already know the segment: + // * is native + // * we have native access + // * there is no cleanup action + // * the segment is read/write + return SegmentFactories.makeNativeSegmentUnchecked(segment.address(), byteSize, + MemorySessionImpl.toMemorySession(arena), false, null); + } + + public static CarrierLocalArenaPools create(long byteSize) { + if (byteSize < 0) { + throw new IllegalArgumentException(); + } + return new CarrierLocalArenaPools(byteSize, 1L); + } + + public static CarrierLocalArenaPools create(long byteSize, + long byteAlignment) { + Utils.checkAllocationSizeAndAlign(byteSize, byteAlignment); + return new CarrierLocalArenaPools(byteSize, byteAlignment); + } + + public static CarrierLocalArenaPools create(MemoryLayout layout) { + Objects.requireNonNull(layout); + return new CarrierLocalArenaPools(layout.byteSize(), layout.byteAlignment()); + } + +} diff --git a/src/java.base/share/classes/jdk/internal/foreign/NoInitSegmentAllocator.java b/src/java.base/share/classes/jdk/internal/foreign/NoInitSegmentAllocator.java new file mode 100644 index 0000000000000..8bb9fb37add8c --- /dev/null +++ b/src/java.base/share/classes/jdk/internal/foreign/NoInitSegmentAllocator.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.internal.foreign; + +import jdk.internal.vm.annotation.ForceInline; + +import java.lang.foreign.SegmentAllocator; + +public interface NoInitSegmentAllocator extends SegmentAllocator { + + NativeMemorySegmentImpl allocateNoInit(long byteSize, long byteAlignment); + + @ForceInline + @Override + default NativeMemorySegmentImpl allocate(long byteSize, long byteAlignment) { + NativeMemorySegmentImpl segment = allocateNoInit(byteSize, byteAlignment); + segment.fill((byte)0); + return segment; + } +} \ No newline at end of file diff --git a/src/java.base/share/classes/jdk/internal/invoke/MhUtil.java b/src/java.base/share/classes/jdk/internal/invoke/MhUtil.java index 16a4e10221c66..d8a950728a246 100644 --- a/src/java.base/share/classes/jdk/internal/invoke/MhUtil.java +++ b/src/java.base/share/classes/jdk/internal/invoke/MhUtil.java @@ -75,4 +75,14 @@ public static MethodHandle findVirtual(MethodHandles.Lookup lookup, } } + public static MethodHandle findStatic(MethodHandles.Lookup lookup, + String name, + MethodType type) { + try { + return lookup.findStatic(lookup.lookupClass(), name, type); + } catch (ReflectiveOperationException e) { + throw new InternalError(e); + } + } + } diff --git a/test/jdk/java/foreign/TestCaptureStateUtil.java b/test/jdk/java/foreign/TestCaptureStateUtil.java new file mode 100644 index 0000000000000..4ed3388bfa294 --- /dev/null +++ b/test/jdk/java/foreign/TestCaptureStateUtil.java @@ -0,0 +1,150 @@ +/* + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @summary Test CaptureStateUtil + * @modules java.base/jdk.internal.foreign + * @run junit TestCaptureStateUtil + */ + +import jdk.internal.foreign.CaptureStateUtil; +import org.junit.jupiter.api.Test; + +import java.lang.foreign.Linker; +import java.lang.foreign.MemoryLayout; +import java.lang.foreign.MemorySegment; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.lang.invoke.VarHandle; + +import static org.junit.jupiter.api.Assertions.*; + +final class TestCaptureStateUtil { + + private static final String ERRNO_NAME = "errno"; + + private static final VarHandle ERRNO_HANDLE = Linker.Option.captureStateLayout() + .varHandle(MemoryLayout.PathElement.groupElement(ERRNO_NAME)); + + private static final MethodHandle INT_DUMMY_HANDLE; + private static final MethodHandle LONG_DUMMY_HANDLE; + + static { + try { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + INT_DUMMY_HANDLE = lookup + .findStatic(TestCaptureStateUtil.class, "dummy", + MethodType.methodType(int.class, MemorySegment.class, int.class, int.class)); + LONG_DUMMY_HANDLE = lookup + .findStatic(TestCaptureStateUtil.class, "dummy", + MethodType.methodType(long.class, MemorySegment.class, long.class, int.class)); + } catch (ReflectiveOperationException e) { + throw new InternalError(e); + } + } + + private static final MethodHandle ADAPTED_INT = CaptureStateUtil + .adaptSystemCall(INT_DUMMY_HANDLE, ERRNO_NAME); + private static final MethodHandle ADAPTED_LONG = CaptureStateUtil + .adaptSystemCall(LONG_DUMMY_HANDLE, ERRNO_NAME); + + @Test + void successfulInt() throws Throwable { + int r = (int) ADAPTED_INT.invokeExact(1, 0); + assertEquals(1, r); + } + + private static final int EACCES = 13; /* Permission denied */ + + @Test + void errorInt() throws Throwable { + int r = (int) ADAPTED_INT.invokeExact(-1, EACCES); + assertEquals(-EACCES, r); + } + + @Test + void successfulLong() throws Throwable { + long r = (long) ADAPTED_LONG.invokeExact(1L, 0); + assertEquals(1, r); + } + + @Test + void errorLong() throws Throwable { + long r = (long) ADAPTED_LONG.invokeExact(-1L, EACCES); + assertEquals(-EACCES, r); + } + + @Test + void successfulIntPerHandle() throws Throwable { + MethodHandle handle = CaptureStateUtil + .adaptSystemCall(INT_DUMMY_HANDLE, ERRNO_NAME); + int r = (int) handle.invokeExact(1, 0); + assertEquals(1, r); + } + + @Test + void invariants() throws Throwable { + MethodHandle noSegment = MethodHandles.lookup() + .findStatic(TestCaptureStateUtil.class, "wrongType", + MethodType.methodType(long.class, long.class, int.class)); + + var noSegEx = assertThrows(IllegalArgumentException.class, () -> CaptureStateUtil.adaptSystemCall(noSegment, ERRNO_NAME)); + assertTrue(noSegEx.getMessage().contains("does not have a MemorySegment as the first parameter")); + + MethodHandle wrongReturnType = MethodHandles.lookup() + .findStatic(TestCaptureStateUtil.class, "wrongType", + MethodType.methodType(short.class, MemorySegment.class, long.class, int.class)); + + var wrongRetEx = assertThrows(IllegalArgumentException.class, () -> CaptureStateUtil.adaptSystemCall(wrongReturnType, ERRNO_NAME)); + assertTrue(wrongRetEx.getMessage().contains("does not return an int or a long")); + + var wrongCaptureName = assertThrows(IllegalArgumentException.class, () -> CaptureStateUtil.adaptSystemCall(LONG_DUMMY_HANDLE, "foo")); + assertTrue(wrongCaptureName.getMessage().startsWith("Unknown state name: foo"), wrongCaptureName.getMessage()); + + assertThrows(NullPointerException.class, () -> CaptureStateUtil.adaptSystemCall(null, ERRNO_NAME)); + assertThrows(NullPointerException.class, () -> CaptureStateUtil.adaptSystemCall(noSegment, null)); + } + + // Dummy method that is just returning the provided parameters + private static int dummy(MemorySegment segment, int result, int errno) { + ERRNO_HANDLE.set(segment, 0, errno); + return result; + } + + // Dummy method that is just returning the provided parameters + private static long dummy(MemorySegment segment, long result, int errno) { + ERRNO_HANDLE.set(segment, 0, errno); + return result; + } + + private static long wrongType(long result, int errno) { + return 0; + } + + private static short wrongType(MemorySegment segment, long result, int errno) { + return 0; + } + +} \ No newline at end of file diff --git a/test/jdk/java/foreign/TestCarrierLocalArenaPools.java b/test/jdk/java/foreign/TestCarrierLocalArenaPools.java new file mode 100644 index 0000000000000..2f89c04b88db3 --- /dev/null +++ b/test/jdk/java/foreign/TestCarrierLocalArenaPools.java @@ -0,0 +1,295 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +/* + * @test + * @summary Test TestCarrierLocalArenaPools + * @library /test/lib + * @modules java.base/jdk.internal.foreign + * @run junit TestCarrierLocalArenaPools + */ + +import java.lang.foreign.Arena; +import java.lang.foreign.Linker; +import java.lang.foreign.MemoryLayout; +import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.function.Consumer; +import java.util.stream.Stream; + +import jdk.internal.foreign.CarrierLocalArenaPools; +import jdk.test.lib.thread.VThreadRunner; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static java.lang.foreign.ValueLayout.JAVA_BYTE; +import static org.junit.jupiter.api.Assertions.*; + +final class TestCarrierLocalArenaPools { + + private static final long POOL_SIZE = 64; + private static final long SMALL_ALLOC_SIZE = 8; + private static final long VERY_LARGE_ALLOC_SIZE = 1L << 10; + + @Test + void invariants1LongArg() { + assertThrows(IllegalArgumentException.class, () -> CarrierLocalArenaPools.create(-1)); + CarrierLocalArenaPools pool = CarrierLocalArenaPools.create(0); + try (var arena = pool.take()) { + // This should come from the underlying arena and not from recyclable memory + assertDoesNotThrow(() -> arena.allocate(1)); + try (var arena2 = pool.take()) { + assertDoesNotThrow(() -> arena.allocate(1)); + } + } + } + + @Test + void invariants2LongArgs() { + assertThrows(IllegalArgumentException.class, () -> CarrierLocalArenaPools.create(-1, 2)); + assertThrows(IllegalArgumentException.class, () -> CarrierLocalArenaPools.create(1, -1)); + assertThrows(IllegalArgumentException.class, () -> CarrierLocalArenaPools.create(1, 3)); + CarrierLocalArenaPools pool = CarrierLocalArenaPools.create(0, 16); + try (var arena = pool.take()) { + // This should come from the underlying arena and not from recyclable memory + assertDoesNotThrow(() -> arena.allocate(1)); + try (var arena2 = pool.take()) { + assertDoesNotThrow(() -> arena.allocate(1)); + } + } + } + + @ParameterizedTest + @MethodSource("pools") + void negativeAlloc(CarrierLocalArenaPools pool) { + Consumer action = arena -> + assertThrows(IllegalArgumentException.class, () -> arena.allocate(-1)); + doInTwoStackedArenas(pool, action, action); + } + + @ParameterizedTest + @MethodSource("pools") + void negativeAllocVt(CarrierLocalArenaPools pool) { + VThreadRunner.run(() -> negativeAlloc(pool)); + } + + @ParameterizedTest + @MethodSource("pools") + void allocateConfinement(CarrierLocalArenaPools pool) { + Consumer allocateAction = arena -> + assertThrows(WrongThreadException.class, () -> { + CompletableFuture future = CompletableFuture.supplyAsync(pool::take); + var otherThreadArena = future.get(); + otherThreadArena.allocate(SMALL_ALLOC_SIZE); + // Intentionally do not close the otherThreadArena here. + }); + doInTwoStackedArenas(pool, allocateAction, allocateAction); + } + + @ParameterizedTest + @MethodSource("pools") + void allocateConfinementVt(CarrierLocalArenaPools pool) { + VThreadRunner.run(() -> allocateConfinement(pool)); + } + + @ParameterizedTest + @MethodSource("pools") + void closeConfinement(CarrierLocalArenaPools pool) { + Consumer closeAction = arena -> { + CompletableFuture future = CompletableFuture.supplyAsync(pool::take); + Arena otherThreadArena = null; + try { + otherThreadArena = future.get(); + } catch (InterruptedException | ExecutionException e) { + fail(e); + } + assertThrows(WrongThreadException.class, otherThreadArena::close); + }; + doInTwoStackedArenas(pool, closeAction, closeAction); + } + + @ParameterizedTest + @MethodSource("pools") + void closeConfinementVt(CarrierLocalArenaPools pool) { + VThreadRunner.run(() -> closeConfinement(pool)); + } + + @ParameterizedTest + @MethodSource("pools") + void reuse(CarrierLocalArenaPools pool) { + MemorySegment firstSegment; + MemorySegment secondSegment; + try (var arena = pool.take()) { + firstSegment = arena.allocate(SMALL_ALLOC_SIZE); + } + try (var arena = pool.take()) { + secondSegment = arena.allocate(SMALL_ALLOC_SIZE); + } + assertNotSame(firstSegment, secondSegment); + assertNotSame(firstSegment.scope(), secondSegment.scope()); + assertEquals(firstSegment.address(), secondSegment.address()); + assertThrows(IllegalStateException.class, () -> firstSegment.get(JAVA_BYTE, 0)); + assertThrows(IllegalStateException.class, () -> secondSegment.get(JAVA_BYTE, 0)); + } + + @ParameterizedTest + @MethodSource("pools") + void reuseVt(CarrierLocalArenaPools pool) { + VThreadRunner.run(() -> reuse(pool)); + } + + @ParameterizedTest + @MethodSource("pools") + void largeAlloc(CarrierLocalArenaPools pool) { + try (var arena = pool.take()) { + var segment = arena.allocate(VERY_LARGE_ALLOC_SIZE); + assertEquals(VERY_LARGE_ALLOC_SIZE, segment.byteSize()); + } + } + + @ParameterizedTest + @MethodSource("pools") + void largeAllocSizeVt(CarrierLocalArenaPools pool) { + VThreadRunner.run(() -> largeAlloc(pool)); + } + + @Test + void allocationSameAsPoolSize() { + var pool = CarrierLocalArenaPools.create(4); + long firstAddress; + try (var arena = pool.take()) { + var segment = arena.allocate(4); + firstAddress = segment.address(); + } + try (var arena = pool.take()) { + var segment = arena.allocate(4 + 1); + assertNotEquals(firstAddress, segment.address()); + } + for (int i = 0; i < 10; i++) { + try (var arena = pool.take()) { + var segment = arena.allocate(4); + assertEquals(firstAddress, segment.address()); + var segmentTwo = arena.allocate(4); + assertNotEquals(firstAddress, segmentTwo.address()); + } + } + } + + @Test + void allocationCaptureStateLayout() { + var layout = Linker.Option.captureStateLayout(); + var pool = CarrierLocalArenaPools.create(layout); + long firstAddress; + try (var arena = pool.take()) { + var segment = arena.allocate(layout); + firstAddress = segment.address(); + } + for (int i = 0; i < 10; i++) { + try (var arena = pool.take()) { + var segment = arena.allocate(layout); + assertEquals(firstAddress, segment.address()); + var segmentTwo = arena.allocate(layout); + assertNotEquals(firstAddress, segmentTwo.address()); + } + } + } + + @ParameterizedTest + @MethodSource("pools") + void outOfOrderUse(CarrierLocalArenaPools pool) { + Arena firstArena = pool.take(); + Arena secondArena = pool.take(); + firstArena.close(); + Arena thirdArena = pool.take(); + secondArena.close(); + thirdArena.close(); + } + + @ParameterizedTest + @MethodSource("pools") + void zeroing(CarrierLocalArenaPools pool) { + try (var arena = pool.take()) { + var seg = arena.allocate(SMALL_ALLOC_SIZE); + seg.fill((byte) 1); + } + try (var arena = pool.take()) { + var seg = arena.allocate(SMALL_ALLOC_SIZE); + for (int i = 0; i < SMALL_ALLOC_SIZE; i++) { + assertEquals((byte) 0, seg.get(JAVA_BYTE, i)); + } + } + } + + @ParameterizedTest + @MethodSource("pools") + void zeroingVt(CarrierLocalArenaPools pool) { + VThreadRunner.run(() -> zeroing(pool)); + } + + @ParameterizedTest + @MethodSource("pools") + void useAfterFree(CarrierLocalArenaPools pool) { + MemorySegment segment = null; + try (var arena = pool.take()){ + segment = arena.allocate(SMALL_ALLOC_SIZE); + } + final var closedSegment = segment; + var e = assertThrows(IllegalStateException.class, () -> closedSegment.get(ValueLayout.JAVA_INT, 0)); + assertEquals("Already closed", e.getMessage()); + } + + @ParameterizedTest + @MethodSource("pools") + void toStringTest(CarrierLocalArenaPools pool) { + assertTrue(pool.toString().contains("ArenaPool")); + try (var arena = pool.take()) { + assertTrue(arena.toString().contains("SlicingArena")); + } + } + + // Factories and helper methods + + static Stream pools() { + return Stream.of( + CarrierLocalArenaPools.create(POOL_SIZE), + CarrierLocalArenaPools.create(POOL_SIZE, 16), + CarrierLocalArenaPools.create(MemoryLayout.sequenceLayout(POOL_SIZE, JAVA_BYTE)) + ); + } + + static void doInTwoStackedArenas(CarrierLocalArenaPools pool, + Consumer firstAction, + Consumer secondAction) { + try (var firstArena = pool.take()) { + firstAction.accept(firstArena); + try (var secondArena = pool.take()) { + secondAction.accept(secondArena); + } + } + } + +} \ No newline at end of file diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/CaptureStateUtilBench.java b/test/micro/org/openjdk/bench/java/lang/foreign/CaptureStateUtilBench.java new file mode 100644 index 0000000000000..6ba08e50874c8 --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/foreign/CaptureStateUtilBench.java @@ -0,0 +1,130 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package org.openjdk.bench.java.lang.foreign; + +import jdk.internal.foreign.CaptureStateUtil; +import jdk.internal.foreign.CarrierLocalArenaPools; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.lang.foreign.Arena; +import java.lang.foreign.Linker; +import java.lang.foreign.MemoryLayout; +import java.lang.foreign.MemorySegment; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.lang.invoke.VarHandle; +import java.util.concurrent.TimeUnit; + +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@State(Scope.Benchmark) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(value = 3, jvmArgs = {"--add-exports=java.base/jdk.internal.foreign=ALL-UNNAMED", + "--enable-native-access=ALL-UNNAMED"}) +public class CaptureStateUtilBench { + + private static final CarrierLocalArenaPools POOLS = CarrierLocalArenaPools.create(16); + + private static final String ERRNO_NAME = "errno"; + + private static final VarHandle ERRNO_HANDLE = Linker.Option.captureStateLayout() + .varHandle(MemoryLayout.PathElement.groupElement(ERRNO_NAME)); + + private static final long SIZE = Linker.Option.captureStateLayout().byteSize(); + + private static final MethodHandle DUMMY_EXPLICIT_ALLOC = dummyExplicitAlloc(); + private static final MethodHandle DUMMY_TL_ALLOC = dummyTlAlloc(); + + @Benchmark + public int explicitAllocationSuccess() throws Throwable { + try (var arena = Arena.ofConfined()) { + return (int) DUMMY_EXPLICIT_ALLOC.invokeExact(arena.allocate(SIZE), 0, 0); + } + } + + @Benchmark + public int explicitAllocationFail() throws Throwable { + try (var arena = Arena.ofConfined()) { + return (int) DUMMY_EXPLICIT_ALLOC.invokeExact(arena.allocate(SIZE), -1, 1); + } + } + + @Benchmark + public int tlAllocationSuccess() throws Throwable { + try (var arena = POOLS.take()) { + return (int) DUMMY_EXPLICIT_ALLOC.invokeExact(arena.allocate(SIZE), 0, 0); + } + } + + @Benchmark + public int tlAllocationFail() throws Throwable { + try (var arena = POOLS.take()) { + return (int) DUMMY_EXPLICIT_ALLOC.invokeExact(arena.allocate(SIZE), -1, 1); + } + } + + @Benchmark + public int adaptedSysCallSuccess() throws Throwable { + return (int) DUMMY_TL_ALLOC.invokeExact(0, 0); + } + + @Benchmark + public int adaptedSysCallFail() throws Throwable { + return (int) DUMMY_TL_ALLOC.invokeExact( -1, 1); + } + + private static MethodHandle dummyExplicitAlloc() { + try { + return MethodHandles.lookup().findStatic(CaptureStateUtilBench.class, + "dummy", MethodType.methodType(int.class, MemorySegment.class, int.class, int.class)); + } catch (ReflectiveOperationException roe) { + throw new RuntimeException(roe); + } + } + + private static MethodHandle dummyTlAlloc() { + final MethodHandle handle = dummyExplicitAlloc(); + return CaptureStateUtil.adaptSystemCall(handle, ERRNO_NAME); + } + + // Dummy method that is just returning the provided parameters + private static int dummy(MemorySegment segment, int result, int errno) { + if (errno != 0) { + // Assuming the capture state is only modified upon detecting an error. + ERRNO_HANDLE.set(segment, 0, errno); + } + return result; + } + +} \ No newline at end of file From ea4a3c337f71475a4b0c833babd7d0fbe69f622e Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Fri, 7 Feb 2025 15:13:25 +0000 Subject: [PATCH 02/12] Add benchmarks --- .../java/lang/foreign/ArenaPoolBench.java | 75 ++++++++++++++++ .../java/lang/foreign/ArenaPoolFromBench.java | 88 +++++++++++++++++++ .../lang/foreign/CaptureStateUtilBench.java | 3 + 3 files changed, 166 insertions(+) create mode 100644 test/micro/org/openjdk/bench/java/lang/foreign/ArenaPoolBench.java create mode 100644 test/micro/org/openjdk/bench/java/lang/foreign/ArenaPoolFromBench.java diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/ArenaPoolBench.java b/test/micro/org/openjdk/bench/java/lang/foreign/ArenaPoolBench.java new file mode 100644 index 0000000000000..e4463e54fbdcb --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/foreign/ArenaPoolBench.java @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +package org.openjdk.bench.java.lang.foreign; + +import jdk.internal.foreign.CarrierLocalArenaPools; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.lang.foreign.Arena; +import java.util.concurrent.TimeUnit; + +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@State(Scope.Thread) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(value = 3, jvmArgs = { + "--add-exports=java.base/jdk.internal.foreign=ALL-UNNAMED", + "--enable-native-access=ALL-UNNAMED"}) +public class ArenaPoolBench { + + private static final CarrierLocalArenaPools POOLS = CarrierLocalArenaPools.create(64, 8); + + @Param({"4", "16", "64", "512"}) + public int ELEM_SIZE; + + @Benchmark + public long confined() { + try (var arena = Arena.ofConfined()) { + return arena.allocate(ELEM_SIZE).address(); + } + } + + @Benchmark + public long pooled() { + try (var arena = POOLS.take()) { + return arena.allocate(ELEM_SIZE).address(); + } + } + + @Fork(value = 3, jvmArgsAppend = "-Djmh.executor=VIRTUAL") + public static class OfVirtual extends ArenaPoolBench { + } + +} diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/ArenaPoolFromBench.java b/test/micro/org/openjdk/bench/java/lang/foreign/ArenaPoolFromBench.java new file mode 100644 index 0000000000000..68bef1593f55f --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/foreign/ArenaPoolFromBench.java @@ -0,0 +1,88 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +package org.openjdk.bench.java.lang.foreign; + +import jdk.internal.foreign.CarrierLocalArenaPools; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.lang.foreign.Arena; +import java.lang.foreign.ValueLayout; +import java.util.concurrent.TimeUnit; + +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@State(Scope.Thread) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(value = 3, jvmArgs = { + "--add-exports=java.base/jdk.internal.foreign=ALL-UNNAMED", + "--enable-native-access=ALL-UNNAMED"}) +public class ArenaPoolFromBench { + + private static final CarrierLocalArenaPools POOLS = CarrierLocalArenaPools.create(32); + + private static final String TEXT = "The quick brown fox"; + + @Benchmark + public long confinedInt() { + try (var arena = Arena.ofConfined()) { + return arena.allocateFrom(ValueLayout.JAVA_INT, 42).address(); + } + } + + @Benchmark + public long confinedSting() { + try (var arena = Arena.ofConfined()) { + return arena.allocateFrom(TEXT).address(); + } + } + + @Benchmark + public long pooledInt() { + try (var arena = POOLS.take()) { + return arena.allocateFrom(ValueLayout.JAVA_INT, 42).address(); + } + } + + @Benchmark + public long pooledString() { + try (var arena = POOLS.take()) { + return arena.allocateFrom(TEXT).address(); + } + } + + @Fork(value = 3, jvmArgsAppend = "-Djmh.executor=VIRTUAL") + public static class OfVirtual extends ArenaPoolFromBench { + } + +} \ No newline at end of file diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/CaptureStateUtilBench.java b/test/micro/org/openjdk/bench/java/lang/foreign/CaptureStateUtilBench.java index 6ba08e50874c8..8d7a3af3759c0 100644 --- a/test/micro/org/openjdk/bench/java/lang/foreign/CaptureStateUtilBench.java +++ b/test/micro/org/openjdk/bench/java/lang/foreign/CaptureStateUtilBench.java @@ -127,4 +127,7 @@ private static int dummy(MemorySegment segment, int result, int errno) { return result; } + @Fork(value = 3, jvmArgsAppend = "-Djmh.executor=VIRTUAL") + public static class OfVirtual extends CaptureStateUtilBench {} + } \ No newline at end of file From 4acac9e9930de0222df53900ab638f03eb1d2e0d Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Fri, 7 Feb 2025 15:22:23 +0000 Subject: [PATCH 03/12] Bump copyright year --- src/java.base/share/classes/jdk/internal/invoke/MhUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.base/share/classes/jdk/internal/invoke/MhUtil.java b/src/java.base/share/classes/jdk/internal/invoke/MhUtil.java index d8a950728a246..444adccc8186a 100644 --- a/src/java.base/share/classes/jdk/internal/invoke/MhUtil.java +++ b/src/java.base/share/classes/jdk/internal/invoke/MhUtil.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it From c0e58df74d1167083d38cb36e81a2f09d860a5da Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Sun, 9 Feb 2025 15:24:45 +0000 Subject: [PATCH 04/12] Improve comments and formatting --- .../internal/foreign/CaptureStateUtil.java | 131 ++++++++++++------ 1 file changed, 92 insertions(+), 39 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java index da7efd381b43f..19a3d0c209ab1 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java +++ b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java @@ -42,9 +42,8 @@ public final class CaptureStateUtil { - private static final MemoryLayout CAPTURE_LAYOUT = Linker.Option.captureStateLayout(); - //private static final CarrierLocalArenaPools POOL = CarrierLocalArenaPools.create(CAPTURE_LAYOUT); - private static final CarrierLocalArenaPools POOL = CarrierLocalArenaPools.create(16); + private static final StructLayout CAPTURE_LAYOUT = Linker.Option.captureStateLayout(); + private static final CarrierLocalArenaPools POOL = CarrierLocalArenaPools.create(CAPTURE_LAYOUT); private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); @@ -84,27 +83,52 @@ public final class CaptureStateUtil { MhUtil.findVirtual(LOOKUP, Arena.class, "close", MethodType.methodType(void.class)); + // The `INNER_HANDLES` contains the common "templates" that can be reused for all + // adapted method handles. Keeping as much as possible reusable reduces the number + // of combinators needed to form an adapted method handle. + // + // The first-level key of the Map tells which return type the handle has + // (`int` or `long`). The second-level key of the Map tells what is the name of + // the captured state ("errno" (all platforms), "GetLastError", or "WSAGetLastError" + // (the two latest only on Windows): + // // (int.class | long.class) -> // ({"GetLastError" | "WSAGetLastError"} | "errno") -> // MethodHandle private static final Map, Map> INNER_HANDLES; static { - - final StructLayout stateLayout = Linker.Option.captureStateLayout(); final Map, Map> classMap = new HashMap<>(); for (var returnType : new Class[]{int.class, long.class}) { - Map handles = stateLayout + Map handles = CAPTURE_LAYOUT .memberLayouts().stream() .collect(Collectors.toUnmodifiableMap( member -> member.name().orElseThrow(), member -> { - VarHandle vh = stateLayout.varHandle(MemoryLayout.PathElement.groupElement(member.name().orElseThrow())); + VarHandle vh = CAPTURE_LAYOUT.varHandle( + MemoryLayout.PathElement.groupElement( + member.name().orElseThrow())); + // This MH is used to extract the named captured state + // from the capturing `MemorySegment`. // (MemorySegment, long)int MethodHandle intExtractor = vh.toMethodHandle(VarHandle.AccessMode.GET); + // As the MH is already adapted to use the appropriate + // offset, we just insert `0L` for the offset. // (MemorySegment)int intExtractor = MethodHandles.insertArguments(intExtractor, 1, 0L); + // If X is the `returnType` (either `int` or `long`) then + // the code below is equivalent to: + // + // X handle(X returnValue, MemorySegment segment) + // if (returnValue >= 0) { + // // Ignore the segment + // return returnValue; + // } else { + // // ignore the returnValue + // return -(X)intExtractor.invokeExact(segment); + // } + // } if (returnType.equals(int.class)) { // (int, MemorySegment)int return MethodHandles.guardWithTest( @@ -122,16 +146,17 @@ public final class CaptureStateUtil { )); classMap.put(returnType, handles); } + // Use an unmodifiable Map to unlock constant folding capabilities. INNER_HANDLES = Map.copyOf(classMap); } - private CaptureStateUtil() { - } + private CaptureStateUtil() {} /** * {@return a new MethodHandle that adapts the provided {@code target} so that it - * directly returns the same value as the {@code target} if it is - * non-negative, otherwise returns the negated errno} + * directly returns the same value as the {@code target} if it is non-negative, + * otherwise returns the negated captured state defined by the provided + * {@code stateName}} *

* This method is suitable for adapting system-call method handles(e.g. * {@code open()}, {@code read()}, and {@code close()}). Clients can check the return @@ -141,7 +166,8 @@ private CaptureStateUtil() { * static final MethodHandle CAPTURING_OPEN = ... * * // (MemorySegment pathname, int flags)int - * static final MethodHandle OPEN = CaptureStateUtil.adaptSystemCall(Pooling.GLOBAL, CAPTURING_OPEN, "errno"); + * static final MethodHandle OPEN = CaptureStateUtil + * .adaptSystemCall(CAPTURING_OPEN, "errno"); * * try { * int fh = (int)OPEN.invoke(pathName, flags); @@ -153,35 +179,45 @@ private CaptureStateUtil() { * throw new RuntimeException(t); * } * - *} - * For a method handle that takes a MemorySegment and two int parameters using GLOBAL, - * the method combinators are doing the equivalent of: - * + *} For a {@code target} method handle that takes a {@code MemorySegment} and two + * {@code int} parameters and returns an {@code int} value, the method returns a new + * method handle that is doing the equivalent of: + *

* {@snippet lang = java: - * public int invoke(int a, int b) { - * final MemorySegment segment = acquireSegment(); - * final int result = (int) handle.invoke(segment, a, b); - * if (result >= 0) { - * return result; + * private static final MemoryLayout CAPTURE_LAYOUT = + * Linker.Option.captureStateLayout(); + * private static final CarrierLocalArenaPools POOL = + * CarrierLocalArenaPools.create(CAPTURE_LAYOUT); + * + * public int invoke(MethodHandle target, + * String stateName, + * int a, int b) { + * try (var arena = POOL.take()) { + * final MemorySegment segment = arena.allocate(CAPTURE_LAYOUT); + * final int result = (int) handle.invoke(segment, a, b); + * if (result >= 0) { + * return result; + * } + * return -(int) CAPTURE_LAYOUT + * .varHandle(MemoryLayout.PathElement.groupElement(stateName)) + * .get(segment, 0); * } - * return -(int) errorHandle.get(segment); * } *} - * Where {@code handle} is the original method handle with the coordinated - * {@code (MemorySegment, int, int)int} and {@code errnoHandle} is a method handle - * that retrieves the error code from the capturing segment. - * + * except it is more performant. In the above {@code stateName} is the name of the + * captured state (e.g. {@code errno}). The static {@code CAPTURE_LAYOUT} is shared + * across all target method handles adapted by this method. * - * @param target method handle that returns an {@code int} or a {@code long} and has - * a capturing state MemorySegment as its first parameter - * @param stateName the name of the capturing state member layout - * (i.e. "errno","GetLastError", or "WSAGetLastError") + * @param target method handle that returns an {@code int} or a {@code long} and + * has a capturing state MemorySegment as its first parameter + * @param stateName the name of the capturing state member layout (i.e. "errno", + * "GetLastError", or "WSAGetLastError") * @throws IllegalArgumentException if the provided {@code target}'s return type is * not {@code int} or {@code long} * @throws IllegalArgumentException if the provided {@code target}'s first parameter * type is not {@linkplain MemorySegment} - * @throws IllegalArgumentException if the provided {@code stateName} is unknown - * on the current platform + * @throws IllegalArgumentException if the provided {@code stateName} is unknown on + * the current platform */ public static MethodHandle adaptSystemCall(MethodHandle target, String stateName) { @@ -207,6 +243,7 @@ public static MethodHandle adaptSystemCall(MethodHandle target, // Make `target` specific adaptations + // Pre-pend all the parameters from the `target` MH. // (C0=MemorySegment, C1-Cn, MemorySegment)(int|long) inner = MethodHandles.collectArguments(inner, 0, target); @@ -215,24 +252,34 @@ public static MethodHandle adaptSystemCall(MethodHandle target, perm[i] = i; } perm[target.type().parameterCount()] = 0; - // Deduplicate the first and last coordinate and only use the first + // Deduplicate the first and last coordinate and only use the first. // (C0=MemorySegment, C1-Cn)(int|long) inner = MethodHandles.permuteArguments(inner, target.type(), perm); + + // Use an `Arena` for the first argument instead and extract a segment from it. // (C0=Arena, C1-Cn)(int|long) inner = MethodHandles.collectArguments(inner, 0, ALLOCATE_MH); + // Add an identity function for the result of the cleanup action. // ((int|long))(int|long) MethodHandle cleanup = MethodHandles.identity(returnType); + // Add a dummy `Throwable` argument for the cleanup action. + // This means, anything thrown will just be propagated. // (Throwable, (int|long))(int|long) cleanup = MethodHandles.dropArguments(cleanup, 0, Throwable.class); - // (Throwable, (int|long), Arena)(int|long) + // Add the first parameter of the `inner` method handle to the cleanup + // action and invoke `Arena::close` when it is run. // Cleanup does not have to have all parameters. It can have zero or more. + // (Throwable, (int|long), Arena)(int|long) cleanup = MethodHandles.collectArguments(cleanup, 2, ARENA_CLOSE_MH); + // Combine the `inner` and `cleanup` action in a try/finally block. // (Arena, C1-Cn)(int|long) MethodHandle tryFinally = MethodHandles.tryFinally(inner, cleanup); - // Finally we arrive at (C1-Cn)(int|long) + // Acquire the arena from the global pool. + // Finally, we arrive at the intended method handle: + // (C1-Cn)(int|long) return MethodHandles.collectArguments(tryFinally, 0, ACQUIRE_ARENA_MH); } @@ -261,12 +308,15 @@ private static boolean nonNegative(int value) { } @ForceInline - private static int success(int value, MemorySegment segment) { + private static int success(int value, + MemorySegment segment) { return value; } @ForceInline - private static int error(MethodHandle errorHandle, int value, MemorySegment segment) throws Throwable { + private static int error(MethodHandle errorHandle, + int value, + MemorySegment segment) throws Throwable { return -(int) errorHandle.invokeExact(segment); } @@ -276,12 +326,15 @@ private static boolean nonNegative(long value) { } @ForceInline - private static long success(long value, MemorySegment segment) { + private static long success(long value, + MemorySegment segment) { return value; } @ForceInline - private static long error(MethodHandle errorHandle, long value, MemorySegment segment) throws Throwable { + private static long error(MethodHandle errorHandle, + long value, + MemorySegment segment) throws Throwable { return -(int) errorHandle.invokeExact(segment); } From 904f6771cd131aa9a3cb2bd631097e78307e20f1 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Sun, 9 Feb 2025 15:53:25 +0000 Subject: [PATCH 05/12] Use lazy initialization --- .../internal/foreign/CaptureStateUtil.java | 115 +++++++++--------- .../java/foreign/TestCaptureStateUtil.java | 2 +- 2 files changed, 56 insertions(+), 61 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java index 19a3d0c209ab1..f23086c2ae722 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java +++ b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java @@ -35,10 +35,10 @@ import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.invoke.VarHandle; -import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.stream.Collectors; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; public final class CaptureStateUtil { @@ -83,9 +83,10 @@ public final class CaptureStateUtil { MhUtil.findVirtual(LOOKUP, Arena.class, "close", MethodType.methodType(void.class)); - // The `INNER_HANDLES` contains the common "templates" that can be reused for all - // adapted method handles. Keeping as much as possible reusable reduces the number + // The `BASIC_HANDLE_CACHE` contains the common "basic handles" that can be reused for + // all adapted method handles. Keeping as much as possible reusable reduces the number // of combinators needed to form an adapted method handle. + // The sub maps are lazily computed. // // The first-level key of the Map tells which return type the handle has // (`int` or `long`). The second-level key of the Map tells what is the name of @@ -95,60 +96,9 @@ public final class CaptureStateUtil { // (int.class | long.class) -> // ({"GetLastError" | "WSAGetLastError"} | "errno") -> // MethodHandle - private static final Map, Map> INNER_HANDLES; - - static { - final Map, Map> classMap = new HashMap<>(); - for (var returnType : new Class[]{int.class, long.class}) { - Map handles = CAPTURE_LAYOUT - .memberLayouts().stream() - .collect(Collectors.toUnmodifiableMap( - member -> member.name().orElseThrow(), - member -> { - VarHandle vh = CAPTURE_LAYOUT.varHandle( - MemoryLayout.PathElement.groupElement( - member.name().orElseThrow())); - // This MH is used to extract the named captured state - // from the capturing `MemorySegment`. - // (MemorySegment, long)int - MethodHandle intExtractor = vh.toMethodHandle(VarHandle.AccessMode.GET); - // As the MH is already adapted to use the appropriate - // offset, we just insert `0L` for the offset. - // (MemorySegment)int - intExtractor = MethodHandles.insertArguments(intExtractor, 1, 0L); - - // If X is the `returnType` (either `int` or `long`) then - // the code below is equivalent to: - // - // X handle(X returnValue, MemorySegment segment) - // if (returnValue >= 0) { - // // Ignore the segment - // return returnValue; - // } else { - // // ignore the returnValue - // return -(X)intExtractor.invokeExact(segment); - // } - // } - if (returnType.equals(int.class)) { - // (int, MemorySegment)int - return MethodHandles.guardWithTest( - NON_NEGATIVE_INT_MH, - SUCCESS_INT_MH, - ERROR_INT_MH.bindTo(intExtractor)); - } else { - // (long, MemorySegment)long - return MethodHandles.guardWithTest( - NON_NEGATIVE_LONG_MH, - SUCCESS_LONG_MH, - ERROR_LONG_MH.bindTo(intExtractor)); - } - } - )); - classMap.put(returnType, handles); - } - // Use an unmodifiable Map to unlock constant folding capabilities. - INNER_HANDLES = Map.copyOf(classMap); - } + private static final Map, Map> BASIC_HANDLE_CACHE = Map.of( + int.class, new ConcurrentHashMap<>(), + long.class, new ConcurrentHashMap<>()); private CaptureStateUtil() {} @@ -233,9 +183,14 @@ public static MethodHandle adaptSystemCall(MethodHandle target, } // ((int | long), MemorySegment)(int | long) - MethodHandle inner = INNER_HANDLES + MethodHandle inner = BASIC_HANDLE_CACHE .get(returnType) - .get(stateName); + .computeIfAbsent(stateName, new Function<>() { + @Override + public MethodHandle apply(String name) { + return basicHandleFor(returnType, name); + } + }); if (inner == null) { throw new IllegalArgumentException("Unknown state name: " + stateName + ". Known on this platform: " + Linker.Option.captureStateLayout()); @@ -283,6 +238,46 @@ public static MethodHandle adaptSystemCall(MethodHandle target, return MethodHandles.collectArguments(tryFinally, 0, ACQUIRE_ARENA_MH); } + private static MethodHandle basicHandleFor(Class returnType, + String capturedStateName) { + final VarHandle vh = CAPTURE_LAYOUT.varHandle( + MemoryLayout.PathElement.groupElement(capturedStateName)); + // This MH is used to extract the named captured state + // from the capturing `MemorySegment`. + // (MemorySegment, long)int + MethodHandle intExtractor = vh.toMethodHandle(VarHandle.AccessMode.GET); + // As the MH is already adapted to use the appropriate + // offset, we just insert `0L` for the offset. + // (MemorySegment)int + intExtractor = MethodHandles.insertArguments(intExtractor, 1, 0L); + + // If X is the `returnType` (either `int` or `long`) then + // the code below is equivalent to: + // + // X handle(X returnValue, MemorySegment segment) + // if (returnValue >= 0) { + // // Ignore the segment + // return returnValue; + // } else { + // // ignore the returnValue + // return -(X)intExtractor.invokeExact(segment); + // } + // } + if (returnType.equals(int.class)) { + // (int, MemorySegment)int + return MethodHandles.guardWithTest( + NON_NEGATIVE_INT_MH, + SUCCESS_INT_MH, + ERROR_INT_MH.bindTo(intExtractor)); + } else { + // (long, MemorySegment)long + return MethodHandles.guardWithTest( + NON_NEGATIVE_LONG_MH, + SUCCESS_LONG_MH, + ERROR_LONG_MH.bindTo(intExtractor)); + } + } + private static IllegalArgumentException illegalArgDoesNot(MethodHandle target, String info) { return new IllegalArgumentException("The provided target " + target + " does not " + info); diff --git a/test/jdk/java/foreign/TestCaptureStateUtil.java b/test/jdk/java/foreign/TestCaptureStateUtil.java index 4ed3388bfa294..309a5680821bf 100644 --- a/test/jdk/java/foreign/TestCaptureStateUtil.java +++ b/test/jdk/java/foreign/TestCaptureStateUtil.java @@ -121,7 +121,7 @@ void invariants() throws Throwable { assertTrue(wrongRetEx.getMessage().contains("does not return an int or a long")); var wrongCaptureName = assertThrows(IllegalArgumentException.class, () -> CaptureStateUtil.adaptSystemCall(LONG_DUMMY_HANDLE, "foo")); - assertTrue(wrongCaptureName.getMessage().startsWith("Unknown state name: foo"), wrongCaptureName.getMessage()); + assertTrue(wrongCaptureName.getMessage().startsWith("Bad layout path: cannot resolve 'foo' in layout ["), wrongCaptureName.getMessage()); assertThrows(NullPointerException.class, () -> CaptureStateUtil.adaptSystemCall(null, ERRNO_NAME)); assertThrows(NullPointerException.class, () -> CaptureStateUtil.adaptSystemCall(noSegment, null)); From 163cde4da506e75f62bb865a6413904d57e7d119 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Sun, 9 Feb 2025 17:08:56 +0000 Subject: [PATCH 06/12] Fix test issue and polish comments --- .../internal/foreign/CaptureStateUtil.java | 118 ++++++++++-------- .../java/foreign/TestCaptureStateUtil.java | 2 +- 2 files changed, 64 insertions(+), 56 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java index f23086c2ae722..aa51da3200621 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java +++ b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java @@ -47,6 +47,8 @@ public final class CaptureStateUtil { private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); + // The method handles below are bound to static methods residing in this class + private static final MethodHandle NON_NEGATIVE_INT_MH = MhUtil.findStatic(LOOKUP, "nonNegative", MethodType.methodType(boolean.class, int.class)); @@ -86,19 +88,40 @@ public final class CaptureStateUtil { // The `BASIC_HANDLE_CACHE` contains the common "basic handles" that can be reused for // all adapted method handles. Keeping as much as possible reusable reduces the number // of combinators needed to form an adapted method handle. - // The sub maps are lazily computed. - // - // The first-level key of the Map tells which return type the handle has - // (`int` or `long`). The second-level key of the Map tells what is the name of - // the captured state ("errno" (all platforms), "GetLastError", or "WSAGetLastError" - // (the two latest only on Windows): + // The map is lazily computed. // - // (int.class | long.class) -> - // ({"GetLastError" | "WSAGetLastError"} | "errno") -> - // MethodHandle - private static final Map, Map> BASIC_HANDLE_CACHE = Map.of( - int.class, new ConcurrentHashMap<>(), - long.class, new ConcurrentHashMap<>()); + private static final Map BASIC_HANDLE_CACHE = + new ConcurrentHashMap<>(); + + // A key that holds both the `returnType` and the `stateName` needed to lookup a + // specific "basic handle". + // returnType E {int.class | long.class} + // stateName can be anything non-null but should E {"GetLastError" | "WSAGetLastError"} | "errno")} + record BasicKey(Class returnType, String stateName) { + + BasicKey(MethodHandle target, String stateName) { + this(returnType(target), Objects.requireNonNull(stateName)); + } + + static Class returnType(MethodHandle target) { + // Implicit null check + final Class returnType = target.type().returnType(); + + if (!(returnType.equals(int.class) || returnType.equals(long.class))) { + throw illegalArgDoesNot(target, "return an int or a long"); + } + if (target.type().parameterCount() == 0 || target.type().parameterType(0) != MemorySegment.class) { + throw illegalArgDoesNot(target, "have a MemorySegment as the first parameter"); + } + return returnType; + } + + private static IllegalArgumentException illegalArgDoesNot(MethodHandle target, String info) { + return new IllegalArgumentException("The provided target " + target + + " does not " + info); + } + + } private CaptureStateUtil() {} @@ -120,7 +143,7 @@ private CaptureStateUtil() {} * .adaptSystemCall(CAPTURING_OPEN, "errno"); * * try { - * int fh = (int)OPEN.invoke(pathName, flags); + * int fh = (int)OPEN.invokeExact(pathName, flags); * if (fh < 0) { * throw new IOException("Error opening file: errno = " + (-fh)); * } @@ -129,7 +152,9 @@ private CaptureStateUtil() {} * throw new RuntimeException(t); * } * - *} For a {@code target} method handle that takes a {@code MemorySegment} and two + *} + * + * For a {@code target} method handle that takes a {@code MemorySegment} and two * {@code int} parameters and returns an {@code int} value, the method returns a new * method handle that is doing the equivalent of: *

@@ -171,77 +196,65 @@ private CaptureStateUtil() {} */ public static MethodHandle adaptSystemCall(MethodHandle target, String stateName) { - // Implicit null check - final Class returnType = target.type().returnType(); - Objects.requireNonNull(stateName); - - if (!(returnType.equals(int.class) || returnType.equals(long.class))) { - throw illegalArgDoesNot(target, "return an int or a long"); - } - if (target.type().parameterCount() == 0 || target.type().parameterType(0) != MemorySegment.class) { - throw illegalArgDoesNot(target, "have a MemorySegment as the first parameter"); - } + // Invariants checked in the BasicKey record + final BasicKey basicKey = new BasicKey(target, stateName); // ((int | long), MemorySegment)(int | long) - MethodHandle inner = BASIC_HANDLE_CACHE - .get(returnType) - .computeIfAbsent(stateName, new Function<>() { + final MethodHandle basicHandle = BASIC_HANDLE_CACHE + // Do not use a lambda to allow early use in the init sequence + .computeIfAbsent(basicKey, new Function<>() { @Override - public MethodHandle apply(String name) { - return basicHandleFor(returnType, name); + public MethodHandle apply(BasicKey basicKey) { + return basicHandleFor(basicKey); } }); - if (inner == null) { - throw new IllegalArgumentException("Unknown state name: " + stateName + - ". Known on this platform: " + Linker.Option.captureStateLayout()); - } - // Make `target` specific adaptations + // Make `target` specific adaptations of the basic handle // Pre-pend all the parameters from the `target` MH. // (C0=MemorySegment, C1-Cn, MemorySegment)(int|long) - inner = MethodHandles.collectArguments(inner, 0, target); + MethodHandle innerAdapted = MethodHandles.collectArguments(basicHandle, 0, target); - int[] perm = new int[target.type().parameterCount() + 1]; + final int[] perm = new int[target.type().parameterCount() + 1]; for (int i = 0; i < target.type().parameterCount(); i++) { perm[i] = i; } + // Last takes first perm[target.type().parameterCount()] = 0; - // Deduplicate the first and last coordinate and only use the first. + // Deduplicate the first and last coordinate and only use the first one. // (C0=MemorySegment, C1-Cn)(int|long) - inner = MethodHandles.permuteArguments(inner, target.type(), perm); + innerAdapted = MethodHandles.permuteArguments(innerAdapted, target.type(), perm); // Use an `Arena` for the first argument instead and extract a segment from it. // (C0=Arena, C1-Cn)(int|long) - inner = MethodHandles.collectArguments(inner, 0, ALLOCATE_MH); + innerAdapted = MethodHandles.collectArguments(innerAdapted, 0, ALLOCATE_MH); // Add an identity function for the result of the cleanup action. // ((int|long))(int|long) - MethodHandle cleanup = MethodHandles.identity(returnType); + MethodHandle cleanup = MethodHandles.identity(basicKey.returnType()); // Add a dummy `Throwable` argument for the cleanup action. // This means, anything thrown will just be propagated. // (Throwable, (int|long))(int|long) cleanup = MethodHandles.dropArguments(cleanup, 0, Throwable.class); - // Add the first parameter of the `inner` method handle to the cleanup - // action and invoke `Arena::close` when it is run. - // Cleanup does not have to have all parameters. It can have zero or more. + // Add the first `Arena` parameter of the `innerAdapted` method handle to the + // cleanup action and invoke `Arena::close` when it is run. The `cleanup` handle + // does not have to have all parameters. It can have zero or more. // (Throwable, (int|long), Arena)(int|long) cleanup = MethodHandles.collectArguments(cleanup, 2, ARENA_CLOSE_MH); - // Combine the `inner` and `cleanup` action in a try/finally block. + // Combine the `innerAdapted` and `cleanup` action into a try/finally block. // (Arena, C1-Cn)(int|long) - MethodHandle tryFinally = MethodHandles.tryFinally(inner, cleanup); + final MethodHandle tryFinally = MethodHandles.tryFinally(innerAdapted, cleanup); // Acquire the arena from the global pool. - // Finally, we arrive at the intended method handle: + // With this, we finally arrive at the intended method handle: // (C1-Cn)(int|long) return MethodHandles.collectArguments(tryFinally, 0, ACQUIRE_ARENA_MH); } - private static MethodHandle basicHandleFor(Class returnType, - String capturedStateName) { + private static MethodHandle basicHandleFor(BasicKey basicKey) { final VarHandle vh = CAPTURE_LAYOUT.varHandle( - MemoryLayout.PathElement.groupElement(capturedStateName)); + MemoryLayout.PathElement.groupElement(basicKey.stateName())); // This MH is used to extract the named captured state // from the capturing `MemorySegment`. // (MemorySegment, long)int @@ -263,7 +276,7 @@ private static MethodHandle basicHandleFor(Class returnType, // return -(X)intExtractor.invokeExact(segment); // } // } - if (returnType.equals(int.class)) { + if (basicKey.returnType().equals(int.class)) { // (int, MemorySegment)int return MethodHandles.guardWithTest( NON_NEGATIVE_INT_MH, @@ -278,11 +291,6 @@ private static MethodHandle basicHandleFor(Class returnType, } } - private static IllegalArgumentException illegalArgDoesNot(MethodHandle target, String info) { - return new IllegalArgumentException("The provided target " + target - + " does not " + info); - } - // The methods below are reflective used via static MethodHandles @ForceInline diff --git a/test/jdk/java/foreign/TestCaptureStateUtil.java b/test/jdk/java/foreign/TestCaptureStateUtil.java index 309a5680821bf..f024a70be5626 100644 --- a/test/jdk/java/foreign/TestCaptureStateUtil.java +++ b/test/jdk/java/foreign/TestCaptureStateUtil.java @@ -124,7 +124,7 @@ void invariants() throws Throwable { assertTrue(wrongCaptureName.getMessage().startsWith("Bad layout path: cannot resolve 'foo' in layout ["), wrongCaptureName.getMessage()); assertThrows(NullPointerException.class, () -> CaptureStateUtil.adaptSystemCall(null, ERRNO_NAME)); - assertThrows(NullPointerException.class, () -> CaptureStateUtil.adaptSystemCall(noSegment, null)); + assertThrows(IllegalArgumentException.class, () -> CaptureStateUtil.adaptSystemCall(noSegment, null)); } // Dummy method that is just returning the provided parameters From 1c2b437e2775965acb7fc0efdbec814d26ecaf74 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Sun, 9 Feb 2025 17:19:11 +0000 Subject: [PATCH 07/12] Clean up --- .../jdk/internal/foreign/CaptureStateUtil.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java index aa51da3200621..ddeb57111ceab 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java +++ b/src/java.base/share/classes/jdk/internal/foreign/CaptureStateUtil.java @@ -40,6 +40,10 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; +/** + * An internal utility class that can be used to adapt system-call-styled method handles + * for efficient and easy use. + */ public final class CaptureStateUtil { private static final StructLayout CAPTURE_LAYOUT = Linker.Option.captureStateLayout(); @@ -93,8 +97,8 @@ public final class CaptureStateUtil { private static final Map BASIC_HANDLE_CACHE = new ConcurrentHashMap<>(); - // A key that holds both the `returnType` and the `stateName` needed to lookup a - // specific "basic handle". + // A key that holds both the `returnType` and the `stateName` needed to look up a + // specific "basic handle" in the `BASIC_HANDLE_CACHE`. // returnType E {int.class | long.class} // stateName can be anything non-null but should E {"GetLastError" | "WSAGetLastError"} | "errno")} record BasicKey(Class returnType, String stateName) { @@ -201,7 +205,9 @@ public static MethodHandle adaptSystemCall(MethodHandle target, // ((int | long), MemorySegment)(int | long) final MethodHandle basicHandle = BASIC_HANDLE_CACHE - // Do not use a lambda to allow early use in the init sequence + // Do not use a lambda in order to allow early use in the init sequence + // This is equivalent to: + // computeIfAbsent(basicKey, CaptureStateUtil::basicHandleFor); .computeIfAbsent(basicKey, new Function<>() { @Override public MethodHandle apply(BasicKey basicKey) { From 0e65ea5b31306c7d2c56c40fa1d21b0a72df4cf0 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Mon, 10 Feb 2025 13:05:37 +0100 Subject: [PATCH 08/12] Add stress test --- .../foreign/TestCarrierLocalArenaPools.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/jdk/java/foreign/TestCarrierLocalArenaPools.java b/test/jdk/java/foreign/TestCarrierLocalArenaPools.java index 2f89c04b88db3..2f52664672a48 100644 --- a/test/jdk/java/foreign/TestCarrierLocalArenaPools.java +++ b/test/jdk/java/foreign/TestCarrierLocalArenaPools.java @@ -35,9 +35,15 @@ import java.lang.foreign.MemoryLayout; import java.lang.foreign.MemorySegment; import java.lang.foreign.ValueLayout; +import java.lang.invoke.VarHandle; +import java.time.Duration; +import java.util.Arrays; +import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.locks.LockSupport; import java.util.function.Consumer; +import java.util.stream.IntStream; import java.util.stream.Stream; import jdk.internal.foreign.CarrierLocalArenaPools; @@ -47,6 +53,8 @@ import org.junit.jupiter.params.provider.MethodSource; import static java.lang.foreign.ValueLayout.JAVA_BYTE; +import static java.lang.foreign.ValueLayout.JAVA_LONG; +import static java.time.temporal.ChronoUnit.SECONDS; import static org.junit.jupiter.api.Assertions.*; final class TestCarrierLocalArenaPools { @@ -271,6 +279,43 @@ void toStringTest(CarrierLocalArenaPools pool) { } } + private static final VarHandle LONG_HANDLE = JAVA_LONG.varHandle(); + + /** + * The objective of this test is to try to provoke a situation where threads are + * competing to use allocated pooled memory and then trying to make sure no thread + * can see the same shared memory another thread is using. + */ + @Test + void stress() throws InterruptedException { + // Just use one pool variant as testing here is fairly expensive. + final CarrierLocalArenaPools pool = pools().limit(1).findFirst().orElseThrow(); + // Make sure it works for both virtual and platform threads (as they are handled differently) + for (var threadBuilder : List.of(Thread.ofVirtual(), Thread.ofPlatform())) { + final Thread[] threads = IntStream.range(0, 1024).mapToObj(_ -> + threadBuilder.start(() -> { + final long threadId = Thread.currentThread().threadId(); + while (!Thread.interrupted()) { + for (int i = 0; i < 1_000_000; i++) { + try (Arena arena = pool.take()) { + // Try to assert no two threads get allocated the same memory region. + final MemorySegment segment = arena.allocate(JAVA_LONG); + LONG_HANDLE.setVolatile(segment, 0L, threadId); + assertEquals(threadId, (long) LONG_HANDLE.getVolatile(segment, 0L)); + } + } + Thread.yield(); // make sure the driver thread gets a chance. + } + })).toArray(Thread[]::new); + Thread.sleep(Duration.of(5, SECONDS)); + Arrays.stream(threads).forEach( + thread -> { + assertTrue(thread.isAlive()); + thread.interrupt(); + }); + } + } + // Factories and helper methods static Stream pools() { From 2fb7af12f5d3e6d08d07bc8c1469c6be82c22cb8 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Mon, 10 Feb 2025 15:12:25 +0100 Subject: [PATCH 09/12] Add test --- test/jdk/java/foreign/TestCaptureStateUtil.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/jdk/java/foreign/TestCaptureStateUtil.java b/test/jdk/java/foreign/TestCaptureStateUtil.java index f024a70be5626..c0a6f750c6188 100644 --- a/test/jdk/java/foreign/TestCaptureStateUtil.java +++ b/test/jdk/java/foreign/TestCaptureStateUtil.java @@ -113,6 +113,10 @@ void invariants() throws Throwable { var noSegEx = assertThrows(IllegalArgumentException.class, () -> CaptureStateUtil.adaptSystemCall(noSegment, ERRNO_NAME)); assertTrue(noSegEx.getMessage().contains("does not have a MemorySegment as the first parameter")); + MethodHandle noArgMH = MethodHandles.empty(MethodType.methodType(int.class)); + var emptyEx = assertThrows(IllegalArgumentException.class, () -> CaptureStateUtil.adaptSystemCall(noArgMH, ERRNO_NAME)); + assertTrue(emptyEx.getMessage().contains("does not have a MemorySegment as the first parameter")); + MethodHandle wrongReturnType = MethodHandles.lookup() .findStatic(TestCaptureStateUtil.class, "wrongType", MethodType.methodType(short.class, MemorySegment.class, long.class, int.class)); From df2bcbdd74b7e19f4629020bdd83614560a20c41 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Tue, 11 Feb 2025 16:21:19 +0100 Subject: [PATCH 10/12] Use an automatic arena --- .../foreign/CarrierLocalArenaPools.java | 33 +++++++++----- .../foreign/TestCarrierLocalArenaPools.java | 43 +++++++++++++++---- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java b/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java index ddfe6199d2184..138b346fa1819 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java +++ b/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java @@ -48,6 +48,8 @@ private CarrierLocalArenaPools(long byteSize, long byteAlignment) { private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess(); + // This method can be invoked by either a virtual thread, a platform thread + // , or a carrier thread (e.g. ForkJoinPool-1-worker-1). @Override protected LocalArenaPoolImpl initialValue() { if (JLA.currentCarrierThread() instanceof CarrierThread) { @@ -69,9 +71,13 @@ protected LocalArenaPoolImpl initialValue() { } } + // This method is never invoked by a virtual thread but can be invoked by + // a platform thread or a carrier thread (e.g. ForkJoinPool-1-worker-1). + // Note: the fork join pool can expand/contract dynamically @Override - protected void threadTerminated(LocalArenaPoolImpl stack) { - stack.close(); + protected void threadTerminated(LocalArenaPoolImpl pool) { + // As we are using Arena.ofAuto, we do not need to explicitly + // close the pool. } }; } @@ -87,8 +93,6 @@ private static sealed abstract class LocalArenaPoolImpl { static final int AVAILABLE = 0; static final int TAKEN = 1; - @Stable - private final Arena pooledArena; @Stable private final MemorySegment segment; // Used both directly and reflectively @@ -96,8 +100,8 @@ private static sealed abstract class LocalArenaPoolImpl { private LocalArenaPoolImpl(long byteSize, long byteAlignment) { - this.pooledArena = Arena.ofConfined(); - this.segment = pooledArena.allocate(byteSize, byteAlignment); + this.segment = Arena.ofAuto() + .allocate(byteSize, byteAlignment); } @ForceInline @@ -108,10 +112,6 @@ public final Arena take() { : arena; } - public final void close() { - pooledArena.close(); - } - /** * {@return {@code true } if the segment was acquired for exclusive use, {@code * false} otherwise} @@ -195,6 +195,8 @@ private final class SlicingArena implements Arena, NoInitSegmentAllocator { private final ArenaImpl delegate; @Stable private final MemorySegment segment; + @Stable + private final Thread owner; private long sp = 0L; @@ -203,6 +205,7 @@ private SlicingArena(ArenaImpl arena, MemorySegment segment) { this.delegate = arena; this.segment = segment; + this.owner = Thread.currentThread(); } @ForceInline @@ -220,6 +223,7 @@ public NativeMemorySegmentImpl allocate(long byteSize, long byteAlignment) { @SuppressWarnings("restricted") @ForceInline public NativeMemorySegmentImpl allocateNoInit(long byteSize, long byteAlignment) { + assertOwnerThread(); final long min = segment.address(); final long start = Utils.alignUp(min + sp, byteAlignment) - min; if (start + byteSize <= segment.byteSize()) { @@ -235,6 +239,7 @@ public NativeMemorySegmentImpl allocateNoInit(long byteSize, long byteAlignment) @ForceInline @Override public void close() { + assertOwnerThread(); delegate.close(); // Intentionally do not releaseSegment() in a finally clause as // the segment still is in play if close() initially fails (e.g. is closed @@ -242,6 +247,14 @@ public void close() { // successfully re-invoked (e.g. from its owner thread). LocalArenaPoolImpl.this.releaseSegment(); } + + @ForceInline + void assertOwnerThread() { + if (owner != Thread.currentThread()) { + throw new WrongThreadException(); + } + } + } } diff --git a/test/jdk/java/foreign/TestCarrierLocalArenaPools.java b/test/jdk/java/foreign/TestCarrierLocalArenaPools.java index 2f52664672a48..250e797d36d6d 100644 --- a/test/jdk/java/foreign/TestCarrierLocalArenaPools.java +++ b/test/jdk/java/foreign/TestCarrierLocalArenaPools.java @@ -40,10 +40,12 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.locks.LockSupport; +import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.stream.IntStream; +import java.util.stream.LongStream; import java.util.stream.Stream; import jdk.internal.foreign.CarrierLocalArenaPools; @@ -128,14 +130,18 @@ void allocateConfinementVt(CarrierLocalArenaPools pool) { @MethodSource("pools") void closeConfinement(CarrierLocalArenaPools pool) { Consumer closeAction = arena -> { - CompletableFuture future = CompletableFuture.supplyAsync(pool::take); - Arena otherThreadArena = null; + // Do not use CompletableFuture here as it might accidentally run on the + // same carrier thread as a virtual thread. + AtomicReference otherThreadArena = new AtomicReference<>(); + var thread = Thread.ofPlatform().start(() -> { + otherThreadArena.set(pool.take()); + }); try { - otherThreadArena = future.get(); - } catch (InterruptedException | ExecutionException e) { - fail(e); + thread.join(); + } catch (InterruptedException ie) { + fail(ie); } - assertThrows(WrongThreadException.class, otherThreadArena::close); + assertThrows(WrongThreadException.class, otherThreadArena.get()::close); }; doInTwoStackedArenas(pool, closeAction, closeAction); } @@ -288,8 +294,27 @@ void toStringTest(CarrierLocalArenaPools pool) { */ @Test void stress() throws InterruptedException { + + // Force the ForkJoin pool to expand/contract so that VT:s will be allocated + // on FJP threads that are later terminated. + long sum = LongStream.range(0, ForkJoinPool.getCommonPoolParallelism() * 2L) + .parallel() + .boxed() + // Using a CompletableFuture expands the FJP + .map(i -> CompletableFuture.supplyAsync(() -> { + try { + TimeUnit.SECONDS.sleep(1); + } catch (InterruptedException ie) { + throw new RuntimeException(ie); + } + return i; + })) + .map(CompletableFuture::join) + .mapToLong(Long::longValue) + .sum(); + // Just use one pool variant as testing here is fairly expensive. - final CarrierLocalArenaPools pool = pools().limit(1).findFirst().orElseThrow(); + final CarrierLocalArenaPools pool = CarrierLocalArenaPools.create(POOL_SIZE); // Make sure it works for both virtual and platform threads (as they are handled differently) for (var threadBuilder : List.of(Thread.ofVirtual(), Thread.ofPlatform())) { final Thread[] threads = IntStream.range(0, 1024).mapToObj(_ -> From 8b9b5087dfdcf18a65a066464aa4d0734633cfea Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 12 Feb 2025 09:42:40 +0100 Subject: [PATCH 11/12] Ensure reachability --- .../foreign/CarrierLocalArenaPools.java | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java b/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java index 138b346fa1819..fccbaa0ca8324 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java +++ b/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java @@ -36,6 +36,7 @@ import java.lang.foreign.Arena; import java.lang.foreign.MemoryLayout; import java.lang.foreign.MemorySegment; +import java.lang.ref.Reference; import java.util.Objects; public final class CarrierLocalArenaPools { @@ -76,8 +77,7 @@ protected LocalArenaPoolImpl initialValue() { // Note: the fork join pool can expand/contract dynamically @Override protected void threadTerminated(LocalArenaPoolImpl pool) { - // As we are using Arena.ofAuto, we do not need to explicitly - // close the pool. + pool.close(); } }; } @@ -93,15 +93,17 @@ private static sealed abstract class LocalArenaPoolImpl { static final int AVAILABLE = 0; static final int TAKEN = 1; + private final Arena arena; @Stable private final MemorySegment segment; + // Used both directly and reflectively int segmentAvailability; private LocalArenaPoolImpl(long byteSize, long byteAlignment) { - this.segment = Arena.ofAuto() - .allocate(byteSize, byteAlignment); + this.arena = Arena.ofAuto(); + this.segment = arena.allocate(byteSize, byteAlignment); } @ForceInline @@ -112,6 +114,17 @@ public final Arena take() { : arena; } + void close() { + // As we are using an automatic arena, we cannot close it here. + // In order to prevent use-after-free issues, we make sure the arena + // is reachable until the dying moments of a carrier thread. The reason for + // this is reinterpreted segments carved out from the `arena` can be used + // independently of the `arena` but are freed when the `arena` is collected. + // Note: carved-out segments can only be used by threads on the + // carrier thread. + Reference.reachabilityFence(arena); + } + /** * {@return {@code true } if the segment was acquired for exclusive use, {@code * false} otherwise} @@ -185,9 +198,6 @@ void releaseSegment() { * segment cannot be used for allocation, a fall-back arena is used instead. This * means allocation never fails due to the size and alignment of the backing * segment. - *

- * Todo: Should we expose a variant of this class as a complement - * to SlicingAllocator? */ private final class SlicingArena implements Arena, NoInitSegmentAllocator { @@ -223,7 +233,6 @@ public NativeMemorySegmentImpl allocate(long byteSize, long byteAlignment) { @SuppressWarnings("restricted") @ForceInline public NativeMemorySegmentImpl allocateNoInit(long byteSize, long byteAlignment) { - assertOwnerThread(); final long min = segment.address(); final long start = Utils.alignUp(min + sp, byteAlignment) - min; if (start + byteSize <= segment.byteSize()) { @@ -258,7 +267,7 @@ void assertOwnerThread() { } } - // Equivalent to: + // Equivalent to but faster than: // return (NativeMemorySegmentImpl) slice // .reinterpret(byteSize, delegate, null); */ @ForceInline From a3af69e53a12b1eb397c1eedc661da4ae19b4732 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 12 Feb 2025 17:01:21 +0100 Subject: [PATCH 12/12] Add holding a reference to the original arena in the taken arena --- .../foreign/CarrierLocalArenaPools.java | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java b/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java index fccbaa0ca8324..9919aaa030824 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java +++ b/src/java.base/share/classes/jdk/internal/foreign/CarrierLocalArenaPools.java @@ -75,10 +75,9 @@ protected LocalArenaPoolImpl initialValue() { // This method is never invoked by a virtual thread but can be invoked by // a platform thread or a carrier thread (e.g. ForkJoinPool-1-worker-1). // Note: the fork join pool can expand/contract dynamically + // We do not use the method here as we are using an automatic arena. @Override - protected void threadTerminated(LocalArenaPoolImpl pool) { - pool.close(); - } + protected void threadTerminated(LocalArenaPoolImpl pool) {} }; } @@ -93,38 +92,29 @@ private static sealed abstract class LocalArenaPoolImpl { static final int AVAILABLE = 0; static final int TAKEN = 1; - private final Arena arena; + // Hold a reference so that the arena is not GC:ed before the thread dies. + @Stable + private final Arena originalArena; @Stable - private final MemorySegment segment; + private final MemorySegment recyclableSegment; // Used both directly and reflectively int segmentAvailability; private LocalArenaPoolImpl(long byteSize, long byteAlignment) { - this.arena = Arena.ofAuto(); - this.segment = arena.allocate(byteSize, byteAlignment); + this.originalArena = Arena.ofAuto(); + this.recyclableSegment = originalArena.allocate(byteSize, byteAlignment); } @ForceInline public final Arena take() { final Arena arena = Arena.ofConfined(); return tryAcquireSegment() - ? new SlicingArena((ArenaImpl) arena, segment) + ? new SlicingArena(originalArena, (ArenaImpl) arena, recyclableSegment) : arena; } - void close() { - // As we are using an automatic arena, we cannot close it here. - // In order to prevent use-after-free issues, we make sure the arena - // is reachable until the dying moments of a carrier thread. The reason for - // this is reinterpreted segments carved out from the `arena` can be used - // independently of the `arena` but are freed when the `arena` is collected. - // Note: carved-out segments can only be used by threads on the - // carrier thread. - Reference.reachabilityFence(arena); - } - /** * {@return {@code true } if the segment was acquired for exclusive use, {@code * false} otherwise} @@ -201,6 +191,19 @@ void releaseSegment() { */ private final class SlicingArena implements Arena, NoInitSegmentAllocator { + // In order to prevent use-after-free issues, we make sure the original arena + // is reachable until the dying moments of a carrier thread AND remains + // reachable whenever a carved out segment can be reached. The reason for + // this is reinterpreted segments carved out from the original arena can be + // used independently of the original arena but are freed when the + // original arena is collected. + // + // To solve this, we also hold a reference to the original arena from which we + // carved out the `segment`. This covers the case when a VT was remounted on + // another CarrierThread and the original CarrierThread + // died and therefore the original arena was not referenced anymore. + @Stable + private final Arena originalArena; @Stable private final ArenaImpl delegate; @Stable @@ -211,9 +214,11 @@ private final class SlicingArena implements Arena, NoInitSegmentAllocator { private long sp = 0L; @ForceInline - private SlicingArena(ArenaImpl arena, + private SlicingArena(Arena originalArena, + ArenaImpl delegate, MemorySegment segment) { - this.delegate = arena; + this.originalArena = originalArena; + this.delegate = delegate; this.segment = segment; this.owner = Thread.currentThread(); } @@ -250,6 +255,8 @@ public NativeMemorySegmentImpl allocateNoInit(long byteSize, long byteAlignment) public void close() { assertOwnerThread(); delegate.close(); + // This is probably not strictly needed but shows intent + Reference.reachabilityFence(originalArena); // Intentionally do not releaseSegment() in a finally clause as // the segment still is in play if close() initially fails (e.g. is closed // from a non-owner thread). Later on the close() method might be