From 4c451110400e08d1b5d0bb5cea7ac1576a73847f Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Mon, 7 Apr 2025 13:49:07 +0200 Subject: [PATCH 01/13] Change set from previous PR --- .../internal/foreign/SlicingAllocator.java | 18 +- .../jdk/internal/foreign/abi/BufferStack.java | 122 ++++++++++++++ .../jdk/internal/foreign/abi/SharedUtils.java | 24 +-- test/jdk/java/foreign/TestBufferStack.java | 157 ++++++++++++++++++ test/jdk/java/foreign/libTestBufferStack.c | 39 +++++ .../lang/foreign/CallOverheadByValue.java | 96 +++++++++++ .../lang/foreign/libCallOverheadByValue.c | 38 +++++ 7 files changed, 474 insertions(+), 20 deletions(-) create mode 100644 src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java create mode 100644 test/jdk/java/foreign/TestBufferStack.java create mode 100644 test/jdk/java/foreign/libTestBufferStack.c create mode 100644 test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java create mode 100644 test/micro/org/openjdk/bench/java/lang/foreign/libCallOverheadByValue.c diff --git a/src/java.base/share/classes/jdk/internal/foreign/SlicingAllocator.java b/src/java.base/share/classes/jdk/internal/foreign/SlicingAllocator.java index db7d476053e54..6b1a071c2af07 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/SlicingAllocator.java +++ b/src/java.base/share/classes/jdk/internal/foreign/SlicingAllocator.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 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 @@ -38,6 +38,22 @@ public SlicingAllocator(MemorySegment segment) { this.segment = segment; } + public long currentOffset() { + return sp; + } + + public void resetTo(long offset) { + if (offset < 0 || offset > sp) + throw new IllegalArgumentException(String.format("offset %d should be in [0, %d] ", offset, sp)); + this.sp = offset; + } + + public boolean canAllocate(long byteSize, long byteAlignment) { + long min = segment.address(); + long start = Utils.alignUp(min + sp, byteAlignment) - min; + return start + byteSize <= segment.byteSize(); + } + MemorySegment trySlice(long byteSize, long byteAlignment) { long min = segment.address(); long start = Utils.alignUp(min + sp, byteAlignment) - min; diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java new file mode 100644 index 0000000000000..150d54856026a --- /dev/null +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java @@ -0,0 +1,122 @@ +/* + * 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.abi; + +import jdk.internal.foreign.SlicingAllocator; +import jdk.internal.misc.CarrierThreadLocal; +import jdk.internal.vm.annotation.ForceInline; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; +import java.lang.foreign.SegmentAllocator; +import java.util.concurrent.locks.ReentrantLock; + +public class BufferStack { + private final long size; + + public BufferStack(long size) { + this.size = size; + } + + private final ThreadLocal tl = new CarrierThreadLocal<>() { + @Override + protected PerThread initialValue() { + return new PerThread(size); + } + }; + + @ForceInline + public Arena pushFrame(long size, long byteAlignment) { + return tl.get().pushFrame(size, byteAlignment); + } + + private static final class PerThread { + private final ReentrantLock lock = new ReentrantLock(); + private final SlicingAllocator stack; + + public PerThread(long size) { + this.stack = new SlicingAllocator(Arena.ofAuto().allocate(size)); + } + + @ForceInline + public Arena pushFrame(long size, long byteAlignment) { + boolean needsLock = Thread.currentThread().isVirtual() && !lock.isHeldByCurrentThread(); + if (needsLock && !lock.tryLock()) { + // Rare: another virtual thread on the same carrier competed for acquisition. + return Arena.ofConfined(); + } + if (!stack.canAllocate(size, byteAlignment)) { + if (needsLock) lock.unlock(); + return Arena.ofConfined(); + } + + return new Frame(needsLock, size, byteAlignment); + } + + private class Frame implements Arena { + private final boolean locked; + private final long parentOffset; + private final long topOfStack; + private final Arena scope = Arena.ofConfined(); + private final SegmentAllocator frame; + + @SuppressWarnings("restricted") + public Frame(boolean locked, long byteSize, long byteAlignment) { + this.locked = locked; + + parentOffset = stack.currentOffset(); + MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); + topOfStack = stack.currentOffset(); + frame = new SlicingAllocator(frameSegment.reinterpret(scope, null)); + } + + private void assertOrder() { + if (topOfStack != stack.currentOffset()) + throw new IllegalStateException("Out of order access: frame not top-of-stack"); + } + + @Override + @SuppressWarnings("restricted") + public MemorySegment allocate(long byteSize, long byteAlignment) { + return frame.allocate(byteSize, byteAlignment); + } + + @Override + public MemorySegment.Scope scope() { + return scope.scope(); + } + + @Override + public void close() { + assertOrder(); + scope.close(); + stack.resetTo(parentOffset); + if (locked) { + lock.unlock(); + } + } + } + } +} \ No newline at end of file diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java b/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java index 125730560a2e6..bf8e8afdc8425 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java @@ -390,26 +390,12 @@ static long pickChunkOffset(long chunkOffset, long byteWidth, int chunkWidth) { : chunkOffset; } - public static Arena newBoundedArena(long size) { - return new Arena() { - final Arena arena = Arena.ofConfined(); - final SegmentAllocator slicingAllocator = SegmentAllocator.slicingAllocator(arena.allocate(size)); - - @Override - public Scope scope() { - return arena.scope(); - } + private static final int LINKER_STACK_SIZE = Integer.getInteger("jdk.internal.foreign.LINKER_STACK_SIZE", 256); + private static final BufferStack LINKER_STACK = new BufferStack(LINKER_STACK_SIZE); - @Override - public void close() { - arena.close(); - } - - @Override - public MemorySegment allocate(long byteSize, long byteAlignment) { - return slicingAllocator.allocate(byteSize, byteAlignment); - } - }; + @ForceInline + public static Arena newBoundedArena(long size) { + return LINKER_STACK.pushFrame(size, 8); } public static Arena newEmptyArena() { diff --git a/test/jdk/java/foreign/TestBufferStack.java b/test/jdk/java/foreign/TestBufferStack.java new file mode 100644 index 0000000000000..bf1ada8854c5b --- /dev/null +++ b/test/jdk/java/foreign/TestBufferStack.java @@ -0,0 +1,157 @@ +/* + * 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 + * @modules java.base/jdk.internal.foreign.abi + * @build NativeTestHelper TestBufferStack + * @run testng/othervm --enable-native-access=ALL-UNNAMED TestBufferStack + */ + +import jdk.internal.foreign.abi.BufferStack; +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.lang.foreign.Arena; +import java.lang.foreign.FunctionDescriptor; +import java.lang.foreign.MemoryLayout; +import java.lang.foreign.MemorySegment; +import java.lang.foreign.SegmentAllocator; +import java.lang.invoke.MethodHandle; +import java.time.Duration; +import java.util.Arrays; +import java.util.stream.IntStream; + +import static java.lang.foreign.MemoryLayout.structLayout; +import static java.lang.foreign.ValueLayout.*; +import static java.time.temporal.ChronoUnit.SECONDS; + +public class TestBufferStack extends NativeTestHelper { + @Test + public void testScopedAllocation() { + int stackSize = 128; + BufferStack stack = new BufferStack(stackSize); + MemorySegment stackSegment; + try (Arena frame1 = stack.pushFrame(3 * JAVA_INT.byteSize(), JAVA_INT.byteAlignment())) { + // Segments have expected sizes and are accessible and allocated consecutively in the same scope. + MemorySegment segment11 = frame1.allocate(JAVA_INT); + Assert.assertEquals(segment11.scope(), frame1.scope()); + Assert.assertEquals(segment11.byteSize(), JAVA_INT.byteSize()); + segment11.set(JAVA_INT, 0, 1); + stackSegment = segment11.reinterpret(stackSize); + + MemorySegment segment12 = frame1.allocate(JAVA_INT); + Assert.assertEquals(segment12.address(), segment11.address() + JAVA_INT.byteSize()); + Assert.assertEquals(segment12.byteSize(), JAVA_INT.byteSize()); + Assert.assertEquals(segment12.scope(), frame1.scope()); + segment12.set(JAVA_INT, 0, 1); + + MemorySegment segment2; + try (Arena frame2 = stack.pushFrame(JAVA_LONG.byteSize(), JAVA_LONG.byteAlignment())) { + Assert.assertNotEquals(frame2.scope(), frame1.scope()); + // same here, but a new scope. + segment2 = frame2.allocate(JAVA_LONG); + Assert.assertEquals(segment2.address(), segment12.address() + /*segment12 size + frame 1 spare + alignment constraint*/ 3 * JAVA_INT.byteSize()); + Assert.assertEquals(segment2.byteSize(), JAVA_LONG.byteSize()); + Assert.assertEquals(segment2.scope(), frame2.scope()); + segment2.set(JAVA_LONG, 0, 1); + + // Frames must be closed in stack order. + Assert.assertThrows(IllegalStateException.class, frame1::close); + } + // Scope is closed here, inner segments throw. + Assert.assertThrows(IllegalStateException.class, () -> segment2.get(JAVA_INT, 0)); + // A new stack frame allocates at the same location (but different scope) as the previous did. + try (Arena frame3 = stack.pushFrame(2 * JAVA_INT.byteSize(), JAVA_INT.byteAlignment())) { + MemorySegment segment3 = frame3.allocate(JAVA_INT); + Assert.assertEquals(segment3.scope(), frame3.scope()); + Assert.assertEquals(segment3.address(), segment12.address() + 2 * JAVA_INT.byteSize()); + } + + // Fallback arena behaves like regular stack frame. + MemorySegment outOfStack; + try (Arena hugeFrame = stack.pushFrame(1024, 4)) { + outOfStack = hugeFrame.allocate(4); + Assert.assertEquals(outOfStack.scope(), hugeFrame.scope()); + Assert.assertTrue(outOfStack.asOverlappingSlice(stackSegment).isEmpty()); + } + Assert.assertThrows(IllegalStateException.class, () -> outOfStack.get(JAVA_INT, 0)); + + // Outer segments are still accessible. + segment11.get(JAVA_INT, 0); + segment12.get(JAVA_INT, 0); + } + } + + @Test + public void stress() throws InterruptedException { + BufferStack stack = new BufferStack(256); + Thread[] vThreads = IntStream.range(0, 1024).mapToObj(_ -> + Thread.ofVirtual().start(() -> { + long threadId = Thread.currentThread().threadId(); + while (!Thread.interrupted()) { + for (int i = 0; i < 1_000_000; i++) { + try (Arena arena = stack.pushFrame(JAVA_LONG.byteSize(), JAVA_LONG.byteAlignment())) { + // Try to assert no two vThreads get allocated the same stack space. + MemorySegment segment = arena.allocate(JAVA_LONG); + JAVA_LONG.varHandle().setVolatile(segment, 0L, threadId); + Assert.assertEquals(threadId, (long) JAVA_LONG.varHandle().getVolatile(segment, 0L)); + } + } + Thread.yield(); // make sure the driver thread gets a chance. + } + })).toArray(Thread[]::new); + Thread.sleep(Duration.of(10, SECONDS)); + Arrays.stream(vThreads).forEach( + thread -> { + Assert.assertTrue(thread.isAlive()); + thread.interrupt(); + }); + } + + static { + System.loadLibrary("TestBufferStack"); + } + + private static final MemoryLayout HVAPoint3D = structLayout(NativeTestHelper.C_DOUBLE, C_DOUBLE, C_DOUBLE); + private static final MemorySegment UPCALL_MH = upcallStub(TestBufferStack.class, "recurse", FunctionDescriptor.of(HVAPoint3D, C_INT)); + private static final MethodHandle DOWNCALL_MH = downcallHandle("recurse", FunctionDescriptor.of(HVAPoint3D, C_INT, ADDRESS)); + + public static MemorySegment recurse(int depth) { + try { + return (MemorySegment) DOWNCALL_MH.invokeExact((SegmentAllocator) Arena.ofAuto(), depth, UPCALL_MH); + } catch (Throwable e) { + throw new RuntimeException(e); + } + } + + @Test + public void testDeepStack() throws Throwable { + // Each downcall and upcall require 48 bytes of stack. + // After five allocations we start falling back. + MemorySegment point = recurse(10); + Assert.assertEquals(point.getAtIndex(C_DOUBLE, 0), 12.0); + Assert.assertEquals(point.getAtIndex(C_DOUBLE, 1), 11.0); + Assert.assertEquals(point.getAtIndex(C_DOUBLE, 2), 10.0); + } +} diff --git a/test/jdk/java/foreign/libTestBufferStack.c b/test/jdk/java/foreign/libTestBufferStack.c new file mode 100644 index 0000000000000..79eb32bf9334c --- /dev/null +++ b/test/jdk/java/foreign/libTestBufferStack.c @@ -0,0 +1,39 @@ +/* + * 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. + */ + +#include "export.h" + +typedef struct { double x, y, z; } HVAPoint3D; + +EXPORT HVAPoint3D recurse(int depth, HVAPoint3D (*cb)(int)) { + if (depth == 0) { + HVAPoint3D result = { 2, 1, 0}; + return result; + } + + HVAPoint3D result = cb(depth - 1); + result.x += 1; + result.y += 1; + result.z += 1; + return result; +} diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java b/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java new file mode 100644 index 0000000000000..8fae1905472ec --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java @@ -0,0 +1,96 @@ +/* + * 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 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.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Warmup; + +import java.lang.foreign.Arena; +import java.lang.foreign.FunctionDescriptor; +import java.lang.foreign.Linker; +import java.lang.foreign.MemoryLayout; +import java.lang.foreign.MemorySegment; +import java.lang.foreign.SegmentAllocator; +import java.lang.foreign.SymbolLookup; +import java.lang.foreign.ValueLayout; +import java.lang.invoke.MethodHandle; +import java.util.concurrent.TimeUnit; + +import static org.openjdk.bench.java.lang.foreign.CLayouts.C_DOUBLE; + +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@State(org.openjdk.jmh.annotations.Scope.Thread) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(value = 3, jvmArgs = { "--enable-native-access=ALL-UNNAMED", "-Djava.library.path=micro/native" }) +public class CallOverheadByValue { + + public static final MemoryLayout POINT_LAYOUT = MemoryLayout.structLayout( + C_DOUBLE, C_DOUBLE + ); + private static final MethodHandle MH_UNIT_BY_VALUE; + private static final MethodHandle MH_UNIT_BY_PTR; + + static { + Linker abi = Linker.nativeLinker(); + System.loadLibrary("CallOverheadByValue"); + SymbolLookup loaderLibs = SymbolLookup.loaderLookup(); + MH_UNIT_BY_VALUE = abi.downcallHandle( + loaderLibs.findOrThrow("unit"), + FunctionDescriptor.of(POINT_LAYOUT) + ); + MH_UNIT_BY_PTR = abi.downcallHandle( + loaderLibs.findOrThrow("unit_ptr"), + FunctionDescriptor.ofVoid(ValueLayout.ADDRESS) + ); + } + + Arena arena = Arena.ofConfined(); + MemorySegment point = arena.allocate(POINT_LAYOUT); + + @TearDown + public void tearDown() { + arena.close(); + } + + @Benchmark + public void byValue() throws Throwable { + // point = unit(); + MemorySegment unused = (MemorySegment) MH_UNIT_BY_VALUE.invokeExact( + (SegmentAllocator) (_, _) -> point); + } + + @Benchmark + public void byPtr() throws Throwable { + // unit_ptr(&point); + MH_UNIT_BY_PTR.invokeExact(point); + } +} diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/libCallOverheadByValue.c b/test/micro/org/openjdk/bench/java/lang/foreign/libCallOverheadByValue.c new file mode 100644 index 0000000000000..2eb80f537d8c8 --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/foreign/libCallOverheadByValue.c @@ -0,0 +1,38 @@ +/* + * 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. + */ + +#include "export.h" + +typedef struct { + double x; + double y; +} DoublePoint; + +EXPORT DoublePoint unit() { + DoublePoint result = { 1, 0 }; + return result; +} + +EXPORT void unit_ptr(DoublePoint* out) { + *out = unit(); +} From 761cd55265665effbc2ca38f87ba7e62c4e42e82 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 23 Apr 2025 15:43:00 +0200 Subject: [PATCH 02/13] Refactor and add benchmark --- .../jdk/internal/foreign/BufferStack.java | 129 ++++++++++++++++++ .../jdk/internal/foreign/abi/BufferStack.java | 122 ----------------- .../jdk/internal/foreign/abi/SharedUtils.java | 4 +- test/jdk/java/foreign/TestBufferStack.java | 8 +- .../java/lang/foreign/BufferStackBench.java | 79 +++++++++++ 5 files changed, 214 insertions(+), 128 deletions(-) create mode 100644 src/java.base/share/classes/jdk/internal/foreign/BufferStack.java delete mode 100644 src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java create mode 100644 test/micro/org/openjdk/bench/java/lang/foreign/BufferStackBench.java diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java new file mode 100644 index 0000000000000..1f9a592162f09 --- /dev/null +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -0,0 +1,129 @@ +/* + * 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.misc.CarrierThreadLocal; +import jdk.internal.vm.annotation.ForceInline; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; +import java.lang.foreign.SegmentAllocator; +import java.lang.ref.Reference; +import java.util.concurrent.locks.ReentrantLock; + +public sealed interface BufferStack { + + Arena pushFrame(long size, long byteAlignment); + + record BufferStackImpl(long size, CarrierThreadLocal tl) implements BufferStack { + + @ForceInline + public Arena pushFrame(long size, long byteAlignment) { + return tl.get().pushFrame(size, byteAlignment); + } + + private static final class PerThread { + private final ReentrantLock lock = new ReentrantLock(); + private final SlicingAllocator stack; + private final Arena arena; + + public PerThread(long size) { + this.arena = Arena.ofAuto(); + this.stack = new SlicingAllocator(arena.allocate(size)); + } + + @ForceInline + public Arena pushFrame(long size, long byteAlignment) { + boolean needsLock = Thread.currentThread().isVirtual() && !lock.isHeldByCurrentThread(); + if (needsLock && !lock.tryLock()) { + // Rare: another virtual thread on the same carrier competed for acquisition. + return Arena.ofConfined(); + } + if (!stack.canAllocate(size, byteAlignment)) { + if (needsLock) lock.unlock(); + return Arena.ofConfined(); + } + + return new Frame(needsLock, size, byteAlignment); + } + + private final class Frame implements Arena { + private final boolean locked; + private final long parentOffset; + private final long topOfStack; + private final Arena scope = Arena.ofConfined(); + private final SegmentAllocator frame; + + @SuppressWarnings("restricted") + public Frame(boolean locked, long byteSize, long byteAlignment) { + this.locked = locked; + + parentOffset = stack.currentOffset(); + MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); + topOfStack = stack.currentOffset(); + frame = new SlicingAllocator(frameSegment.reinterpret(scope, null)); + } + + private void assertOrder() { + if (topOfStack != stack.currentOffset()) + throw new IllegalStateException("Out of order access: frame not top-of-stack"); + } + + @Override + @SuppressWarnings("restricted") + public MemorySegment allocate(long byteSize, long byteAlignment) { + return frame.allocate(byteSize, byteAlignment); + } + + @Override + public MemorySegment.Scope scope() { + return scope.scope(); + } + + @Override + public void close() { + assertOrder(); + scope.close(); + stack.resetTo(parentOffset); + if (locked) { + lock.unlock(); + } + Reference.reachabilityFence(arena); + } + } + } + } + + static BufferStack of(long size) { + return new BufferStackImpl(size, new CarrierThreadLocal<>() { + @Override + protected BufferStackImpl.PerThread initialValue() { + return new BufferStackImpl.PerThread(size); + } + }); + } + +} \ No newline at end of file diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java deleted file mode 100644 index 150d54856026a..0000000000000 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/BufferStack.java +++ /dev/null @@ -1,122 +0,0 @@ -/* - * 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.abi; - -import jdk.internal.foreign.SlicingAllocator; -import jdk.internal.misc.CarrierThreadLocal; -import jdk.internal.vm.annotation.ForceInline; - -import java.lang.foreign.Arena; -import java.lang.foreign.MemorySegment; -import java.lang.foreign.SegmentAllocator; -import java.util.concurrent.locks.ReentrantLock; - -public class BufferStack { - private final long size; - - public BufferStack(long size) { - this.size = size; - } - - private final ThreadLocal tl = new CarrierThreadLocal<>() { - @Override - protected PerThread initialValue() { - return new PerThread(size); - } - }; - - @ForceInline - public Arena pushFrame(long size, long byteAlignment) { - return tl.get().pushFrame(size, byteAlignment); - } - - private static final class PerThread { - private final ReentrantLock lock = new ReentrantLock(); - private final SlicingAllocator stack; - - public PerThread(long size) { - this.stack = new SlicingAllocator(Arena.ofAuto().allocate(size)); - } - - @ForceInline - public Arena pushFrame(long size, long byteAlignment) { - boolean needsLock = Thread.currentThread().isVirtual() && !lock.isHeldByCurrentThread(); - if (needsLock && !lock.tryLock()) { - // Rare: another virtual thread on the same carrier competed for acquisition. - return Arena.ofConfined(); - } - if (!stack.canAllocate(size, byteAlignment)) { - if (needsLock) lock.unlock(); - return Arena.ofConfined(); - } - - return new Frame(needsLock, size, byteAlignment); - } - - private class Frame implements Arena { - private final boolean locked; - private final long parentOffset; - private final long topOfStack; - private final Arena scope = Arena.ofConfined(); - private final SegmentAllocator frame; - - @SuppressWarnings("restricted") - public Frame(boolean locked, long byteSize, long byteAlignment) { - this.locked = locked; - - parentOffset = stack.currentOffset(); - MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); - topOfStack = stack.currentOffset(); - frame = new SlicingAllocator(frameSegment.reinterpret(scope, null)); - } - - private void assertOrder() { - if (topOfStack != stack.currentOffset()) - throw new IllegalStateException("Out of order access: frame not top-of-stack"); - } - - @Override - @SuppressWarnings("restricted") - public MemorySegment allocate(long byteSize, long byteAlignment) { - return frame.allocate(byteSize, byteAlignment); - } - - @Override - public MemorySegment.Scope scope() { - return scope.scope(); - } - - @Override - public void close() { - assertOrder(); - scope.close(); - stack.resetTo(parentOffset); - if (locked) { - lock.unlock(); - } - } - } - } -} \ No newline at end of file diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java b/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java index bf8e8afdc8425..0204906961661 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java @@ -24,9 +24,9 @@ */ package jdk.internal.foreign.abi; -import jdk.internal.access.JavaLangAccess; import jdk.internal.access.JavaLangInvokeAccess; import jdk.internal.access.SharedSecrets; +import jdk.internal.foreign.BufferStack; import jdk.internal.foreign.CABI; import jdk.internal.foreign.abi.AbstractLinker.UpcallStubFactory; import jdk.internal.foreign.abi.aarch64.linux.LinuxAArch64Linker; @@ -391,7 +391,7 @@ static long pickChunkOffset(long chunkOffset, long byteWidth, int chunkWidth) { } private static final int LINKER_STACK_SIZE = Integer.getInteger("jdk.internal.foreign.LINKER_STACK_SIZE", 256); - private static final BufferStack LINKER_STACK = new BufferStack(LINKER_STACK_SIZE); + private static final BufferStack LINKER_STACK = BufferStack.of(LINKER_STACK_SIZE); @ForceInline public static Arena newBoundedArena(long size) { diff --git a/test/jdk/java/foreign/TestBufferStack.java b/test/jdk/java/foreign/TestBufferStack.java index bf1ada8854c5b..a4419581dd624 100644 --- a/test/jdk/java/foreign/TestBufferStack.java +++ b/test/jdk/java/foreign/TestBufferStack.java @@ -23,12 +23,12 @@ /* * @test - * @modules java.base/jdk.internal.foreign.abi + * @modules java.base/jdk.internal.foreign * @build NativeTestHelper TestBufferStack * @run testng/othervm --enable-native-access=ALL-UNNAMED TestBufferStack */ -import jdk.internal.foreign.abi.BufferStack; +import jdk.internal.foreign.BufferStack; import org.testng.Assert; import org.testng.annotations.Test; @@ -50,7 +50,7 @@ public class TestBufferStack extends NativeTestHelper { @Test public void testScopedAllocation() { int stackSize = 128; - BufferStack stack = new BufferStack(stackSize); + BufferStack stack = BufferStack.of(stackSize); MemorySegment stackSegment; try (Arena frame1 = stack.pushFrame(3 * JAVA_INT.byteSize(), JAVA_INT.byteAlignment())) { // Segments have expected sizes and are accessible and allocated consecutively in the same scope. @@ -105,7 +105,7 @@ public void testScopedAllocation() { @Test public void stress() throws InterruptedException { - BufferStack stack = new BufferStack(256); + BufferStack stack = BufferStack.of(256); Thread[] vThreads = IntStream.range(0, 1024).mapToObj(_ -> Thread.ofVirtual().start(() -> { long threadId = Thread.currentThread().threadId(); diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/BufferStackBench.java b/test/micro/org/openjdk/bench/java/lang/foreign/BufferStackBench.java new file mode 100644 index 0000000000000..06f5a3522093b --- /dev/null +++ b/test/micro/org/openjdk/bench/java/lang/foreign/BufferStackBench.java @@ -0,0 +1,79 @@ +/* + * 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.BufferStack; +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.Setup; +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 = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@State(Scope.Thread) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Fork(value = 3, jvmArgs = {"--add-exports=java.base/jdk.internal.foreign=ALL-UNNAMED"}) +public class BufferStackBench { + + @Param({"8", "16", "32"}) + public int ELEM_SIZE; + + private BufferStack bufferStack; + + @Setup + public void setup() { + bufferStack = BufferStack.of(128); + } + + @Benchmark + public long confined() { + try (Arena arena = Arena.ofConfined()) { + return arena.allocate(ELEM_SIZE).address(); + } + } + + @Benchmark + public long buffer() { + try (Arena arena = bufferStack.pushFrame(64, 1)) { + return arena.allocate(ELEM_SIZE).address(); + } + } + + @Fork(value = 3, jvmArgsAppend = "-Djmh.executor=VIRTUAL") + public static class OfVirtual extends BufferStackBench {} + +} + From ef1839466e17c167924c044c6f92f5fc2dc30398 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 23 Apr 2025 16:02:27 +0200 Subject: [PATCH 03/13] Convert class to record and add @ForceInline --- .../jdk/internal/foreign/BufferStack.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java index 1f9a592162f09..d7ad6fd1cbc4d 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -45,15 +45,7 @@ public Arena pushFrame(long size, long byteAlignment) { return tl.get().pushFrame(size, byteAlignment); } - private static final class PerThread { - private final ReentrantLock lock = new ReentrantLock(); - private final SlicingAllocator stack; - private final Arena arena; - - public PerThread(long size) { - this.arena = Arena.ofAuto(); - this.stack = new SlicingAllocator(arena.allocate(size)); - } + private record PerThread(ReentrantLock lock, SlicingAllocator stack, Arena arena) { @ForceInline public Arena pushFrame(long size, long byteAlignment) { @@ -66,10 +58,15 @@ public Arena pushFrame(long size, long byteAlignment) { if (needsLock) lock.unlock(); return Arena.ofConfined(); } - return new Frame(needsLock, size, byteAlignment); } + static PerThread of(long size) { + final Arena arena = Arena.ofAuto(); + final SlicingAllocator stack = new SlicingAllocator(arena.allocate(size)); + return new PerThread(new ReentrantLock(), stack, arena); + } + private final class Frame implements Arena { private final boolean locked; private final long parentOffset; @@ -78,20 +75,22 @@ private final class Frame implements Arena { private final SegmentAllocator frame; @SuppressWarnings("restricted") + @ForceInline public Frame(boolean locked, long byteSize, long byteAlignment) { this.locked = locked; - parentOffset = stack.currentOffset(); MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); topOfStack = stack.currentOffset(); frame = new SlicingAllocator(frameSegment.reinterpret(scope, null)); } + @ForceInline private void assertOrder() { if (topOfStack != stack.currentOffset()) throw new IllegalStateException("Out of order access: frame not top-of-stack"); } + @ForceInline @Override @SuppressWarnings("restricted") public MemorySegment allocate(long byteSize, long byteAlignment) { @@ -103,6 +102,7 @@ public MemorySegment.Scope scope() { return scope.scope(); } + @ForceInline @Override public void close() { assertOrder(); @@ -121,7 +121,7 @@ static BufferStack of(long size) { return new BufferStackImpl(size, new CarrierThreadLocal<>() { @Override protected BufferStackImpl.PerThread initialValue() { - return new BufferStackImpl.PerThread(size); + return BufferStackImpl.PerThread.of(size); } }); } From 1c75c8f1bcc159c57c954dbd5ddadad24162f81f Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 23 Apr 2025 16:34:18 +0200 Subject: [PATCH 04/13] Add rudimentary documentation --- .../classes/jdk/internal/foreign/BufferStack.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java index d7ad6fd1cbc4d..68124ab009a3d 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -34,8 +34,20 @@ import java.lang.ref.Reference; import java.util.concurrent.locks.ReentrantLock; +/** + * A buffer stack that allows efficient reuse of memory segments. This is useful in cases + * where temporary memory is needed. + *

+ * Note: The reused segments are neither zeroed out before nor after re-use. + */ public sealed interface BufferStack { + /** + * {@return a new Arena that tries to provided {@code size} and {@code byteAlignment} + * allocations by recycling the BufferStacks internal memory} + * @param size to be reserved from this BufferStacks internal memory + * @param byteAlignment to be used for reservation + */ Arena pushFrame(long size, long byteAlignment); record BufferStackImpl(long size, CarrierThreadLocal tl) implements BufferStack { From 47f305449f03d369a91fbe52142eac962a7b1884 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 23 Apr 2025 17:18:09 +0200 Subject: [PATCH 05/13] Add benchmark class with virtual executor --- .../bench/java/lang/foreign/CallOverheadByValue.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java b/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java index 8fae1905472ec..ab490a2f3ebcd 100644 --- a/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java +++ b/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java @@ -47,7 +47,7 @@ @BenchmarkMode(Mode.AverageTime) @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) -@Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS) @State(org.openjdk.jmh.annotations.Scope.Thread) @OutputTimeUnit(TimeUnit.NANOSECONDS) @Fork(value = 3, jvmArgs = { "--enable-native-access=ALL-UNNAMED", "-Djava.library.path=micro/native" }) @@ -93,4 +93,8 @@ public void byPtr() throws Throwable { // unit_ptr(&point); MH_UNIT_BY_PTR.invokeExact(point); } + + @Fork(value = 3, jvmArgsAppend = "-Djmh.executor=VIRTUAL") + public static class OfVirtual extends CallOverheadByValue {} + } From 59e1f261c2c2dc2b00895a412e931e8ea5f52ca4 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 23 Apr 2025 18:49:29 +0200 Subject: [PATCH 06/13] Remove interface level --- .../jdk/internal/foreign/BufferStack.java | 141 +++++++++--------- 1 file changed, 69 insertions(+), 72 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java index 68124ab009a3d..61a5b32f6eac8 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -38,104 +38,101 @@ * A buffer stack that allows efficient reuse of memory segments. This is useful in cases * where temporary memory is needed. *

+ * Use the factory {@link #of(long)} to create new instances of this class. + *

* Note: The reused segments are neither zeroed out before nor after re-use. */ -public sealed interface BufferStack { +public record BufferStack(long size, CarrierThreadLocal tl) { /** - * {@return a new Arena that tries to provided {@code size} and {@code byteAlignment} - * allocations by recycling the BufferStacks internal memory} - * @param size to be reserved from this BufferStacks internal memory + * {@return a new Arena that tries to provided {@code size} and {@code byteAlignment} + * allocations by recycling the BufferStacks internal memory} + * + * @param size to be reserved from this BufferStacks internal memory * @param byteAlignment to be used for reservation */ - Arena pushFrame(long size, long byteAlignment); + @ForceInline + public Arena pushFrame(long size, long byteAlignment) { + return tl.get().pushFrame(size, byteAlignment); + } - record BufferStackImpl(long size, CarrierThreadLocal tl) implements BufferStack { + private record PerThread(ReentrantLock lock, SlicingAllocator stack, Arena arena) { @ForceInline public Arena pushFrame(long size, long byteAlignment) { - return tl.get().pushFrame(size, byteAlignment); + boolean needsLock = Thread.currentThread().isVirtual() && !lock.isHeldByCurrentThread(); + if (needsLock && !lock.tryLock()) { + // Rare: another virtual thread on the same carrier competed for acquisition. + return Arena.ofConfined(); + } + if (!stack.canAllocate(size, byteAlignment)) { + if (needsLock) lock.unlock(); + return Arena.ofConfined(); + } + return new Frame(needsLock, size, byteAlignment); + } + + static PerThread of(long size) { + final Arena arena = Arena.ofAuto(); + final SlicingAllocator stack = new SlicingAllocator(arena.allocate(size)); + return new PerThread(new ReentrantLock(), stack, arena); } - private record PerThread(ReentrantLock lock, SlicingAllocator stack, Arena arena) { + private final class Frame implements Arena { + private final boolean locked; + private final long parentOffset; + private final long topOfStack; + private final Arena scope = Arena.ofConfined(); + private final SegmentAllocator frame; + @SuppressWarnings("restricted") @ForceInline - public Arena pushFrame(long size, long byteAlignment) { - boolean needsLock = Thread.currentThread().isVirtual() && !lock.isHeldByCurrentThread(); - if (needsLock && !lock.tryLock()) { - // Rare: another virtual thread on the same carrier competed for acquisition. - return Arena.ofConfined(); - } - if (!stack.canAllocate(size, byteAlignment)) { - if (needsLock) lock.unlock(); - return Arena.ofConfined(); - } - return new Frame(needsLock, size, byteAlignment); + public Frame(boolean locked, long byteSize, long byteAlignment) { + this.locked = locked; + parentOffset = stack.currentOffset(); + MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); + topOfStack = stack.currentOffset(); + frame = new SlicingAllocator(frameSegment.reinterpret(scope, null)); } - static PerThread of(long size) { - final Arena arena = Arena.ofAuto(); - final SlicingAllocator stack = new SlicingAllocator(arena.allocate(size)); - return new PerThread(new ReentrantLock(), stack, arena); + @ForceInline + private void assertOrder() { + if (topOfStack != stack.currentOffset()) + throw new IllegalStateException("Out of order access: frame not top-of-stack"); } - private final class Frame implements Arena { - private final boolean locked; - private final long parentOffset; - private final long topOfStack; - private final Arena scope = Arena.ofConfined(); - private final SegmentAllocator frame; - - @SuppressWarnings("restricted") - @ForceInline - public Frame(boolean locked, long byteSize, long byteAlignment) { - this.locked = locked; - parentOffset = stack.currentOffset(); - MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); - topOfStack = stack.currentOffset(); - frame = new SlicingAllocator(frameSegment.reinterpret(scope, null)); - } - - @ForceInline - private void assertOrder() { - if (topOfStack != stack.currentOffset()) - throw new IllegalStateException("Out of order access: frame not top-of-stack"); - } - - @ForceInline - @Override - @SuppressWarnings("restricted") - public MemorySegment allocate(long byteSize, long byteAlignment) { - return frame.allocate(byteSize, byteAlignment); - } + @ForceInline + @Override + @SuppressWarnings("restricted") + public MemorySegment allocate(long byteSize, long byteAlignment) { + return frame.allocate(byteSize, byteAlignment); + } - @Override - public MemorySegment.Scope scope() { - return scope.scope(); - } + @Override + public MemorySegment.Scope scope() { + return scope.scope(); + } - @ForceInline - @Override - public void close() { - assertOrder(); - scope.close(); - stack.resetTo(parentOffset); - if (locked) { - lock.unlock(); - } - Reference.reachabilityFence(arena); + @ForceInline + @Override + public void close() { + assertOrder(); + scope.close(); + stack.resetTo(parentOffset); + if (locked) { + lock.unlock(); } + Reference.reachabilityFence(arena); } } } - static BufferStack of(long size) { - return new BufferStackImpl(size, new CarrierThreadLocal<>() { + public static BufferStack of(long size) { + return new BufferStack(size, new CarrierThreadLocal<>() { @Override - protected BufferStackImpl.PerThread initialValue() { - return BufferStackImpl.PerThread.of(size); + protected BufferStack.PerThread initialValue() { + return BufferStack.PerThread.of(size); } }); } - -} \ No newline at end of file +} From 1e9d046027db33f5bc749bcb80b90f89193e2f60 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 30 Apr 2025 09:33:44 +0200 Subject: [PATCH 07/13] Break out stress thest and ProblemList it on macosx --- test/jdk/ProblemList.txt | 2 + test/jdk/java/foreign/TestBufferStack.java | 30 -------- .../java/foreign/TestBufferStackStress.java | 72 +++++++++++++++++++ 3 files changed, 74 insertions(+), 30 deletions(-) create mode 100644 test/jdk/java/foreign/TestBufferStackStress.java diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt index c17d8dbe559ec..3edf4a9d18e9a 100644 --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -784,6 +784,8 @@ jdk/jfr/jvm/TestWaste.java 8282427 generic- # jdk_foreign +java/foreign/TestBufferStackStress.java 8350455 macosx-all + ############################################################################ # Client manual tests diff --git a/test/jdk/java/foreign/TestBufferStack.java b/test/jdk/java/foreign/TestBufferStack.java index a4419581dd624..b1e24bcf2970c 100644 --- a/test/jdk/java/foreign/TestBufferStack.java +++ b/test/jdk/java/foreign/TestBufferStack.java @@ -38,13 +38,9 @@ import java.lang.foreign.MemorySegment; import java.lang.foreign.SegmentAllocator; import java.lang.invoke.MethodHandle; -import java.time.Duration; -import java.util.Arrays; -import java.util.stream.IntStream; import static java.lang.foreign.MemoryLayout.structLayout; import static java.lang.foreign.ValueLayout.*; -import static java.time.temporal.ChronoUnit.SECONDS; public class TestBufferStack extends NativeTestHelper { @Test @@ -103,32 +99,6 @@ public void testScopedAllocation() { } } - @Test - public void stress() throws InterruptedException { - BufferStack stack = BufferStack.of(256); - Thread[] vThreads = IntStream.range(0, 1024).mapToObj(_ -> - Thread.ofVirtual().start(() -> { - long threadId = Thread.currentThread().threadId(); - while (!Thread.interrupted()) { - for (int i = 0; i < 1_000_000; i++) { - try (Arena arena = stack.pushFrame(JAVA_LONG.byteSize(), JAVA_LONG.byteAlignment())) { - // Try to assert no two vThreads get allocated the same stack space. - MemorySegment segment = arena.allocate(JAVA_LONG); - JAVA_LONG.varHandle().setVolatile(segment, 0L, threadId); - Assert.assertEquals(threadId, (long) JAVA_LONG.varHandle().getVolatile(segment, 0L)); - } - } - Thread.yield(); // make sure the driver thread gets a chance. - } - })).toArray(Thread[]::new); - Thread.sleep(Duration.of(10, SECONDS)); - Arrays.stream(vThreads).forEach( - thread -> { - Assert.assertTrue(thread.isAlive()); - thread.interrupt(); - }); - } - static { System.loadLibrary("TestBufferStack"); } diff --git a/test/jdk/java/foreign/TestBufferStackStress.java b/test/jdk/java/foreign/TestBufferStackStress.java new file mode 100644 index 0000000000000..757a77cc3839d --- /dev/null +++ b/test/jdk/java/foreign/TestBufferStackStress.java @@ -0,0 +1,72 @@ +/* + * 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 + * @modules java.base/jdk.internal.foreign + * @build NativeTestHelper TestBufferStackStress + * @run testng/othervm --enable-native-access=ALL-UNNAMED TestBufferStackStress + */ + +import jdk.internal.foreign.BufferStack; +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; +import java.time.Duration; +import java.util.Arrays; +import java.util.stream.IntStream; + +import static java.lang.foreign.ValueLayout.*; +import static java.time.temporal.ChronoUnit.SECONDS; + +public class TestBufferStackStress extends NativeTestHelper { + + @Test + public void stress() throws InterruptedException { + BufferStack stack = BufferStack.of(256); + Thread[] vThreads = IntStream.range(0, 1024).mapToObj(_ -> + Thread.ofVirtual().start(() -> { + long threadId = Thread.currentThread().threadId(); + while (!Thread.interrupted()) { + for (int i = 0; i < 1_000_000; i++) { + try (Arena arena = stack.pushFrame(JAVA_LONG.byteSize(), JAVA_LONG.byteAlignment())) { + // Try to assert no two vThreads get allocated the same stack space. + MemorySegment segment = arena.allocate(JAVA_LONG); + JAVA_LONG.varHandle().setVolatile(segment, 0L, threadId); + Assert.assertEquals(threadId, (long) JAVA_LONG.varHandle().getVolatile(segment, 0L)); + } + } + Thread.yield(); // make sure the driver thread gets a chance. + } + })).toArray(Thread[]::new); + Thread.sleep(Duration.of(10, SECONDS)); + Arrays.stream(vThreads).forEach( + thread -> { + Assert.assertTrue(thread.isAlive()); + thread.interrupt(); + }); + } + +} From 101882d5e85231b827aed14d970ca71f066afd38 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 30 Apr 2025 09:46:55 +0200 Subject: [PATCH 08/13] Convert to junit tests --- test/jdk/java/foreign/TestBufferStack.java | 45 ++++++++++--------- .../java/foreign/TestBufferStackStress.java | 10 ++--- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/test/jdk/java/foreign/TestBufferStack.java b/test/jdk/java/foreign/TestBufferStack.java index b1e24bcf2970c..8106024898f26 100644 --- a/test/jdk/java/foreign/TestBufferStack.java +++ b/test/jdk/java/foreign/TestBufferStack.java @@ -25,12 +25,11 @@ * @test * @modules java.base/jdk.internal.foreign * @build NativeTestHelper TestBufferStack - * @run testng/othervm --enable-native-access=ALL-UNNAMED TestBufferStack + * @run junit/othervm --enable-native-access=ALL-UNNAMED TestBufferStack */ import jdk.internal.foreign.BufferStack; -import org.testng.Assert; -import org.testng.annotations.Test; +import org.junit.jupiter.api.Test; import java.lang.foreign.Arena; import java.lang.foreign.FunctionDescriptor; @@ -41,8 +40,10 @@ import static java.lang.foreign.MemoryLayout.structLayout; import static java.lang.foreign.ValueLayout.*; +import static org.junit.jupiter.api.Assertions.*; public class TestBufferStack extends NativeTestHelper { + @Test public void testScopedAllocation() { int stackSize = 128; @@ -51,47 +52,47 @@ public void testScopedAllocation() { try (Arena frame1 = stack.pushFrame(3 * JAVA_INT.byteSize(), JAVA_INT.byteAlignment())) { // Segments have expected sizes and are accessible and allocated consecutively in the same scope. MemorySegment segment11 = frame1.allocate(JAVA_INT); - Assert.assertEquals(segment11.scope(), frame1.scope()); - Assert.assertEquals(segment11.byteSize(), JAVA_INT.byteSize()); + assertEquals(frame1.scope(), segment11.scope()); + assertEquals(JAVA_INT.byteSize(), segment11.byteSize()); segment11.set(JAVA_INT, 0, 1); stackSegment = segment11.reinterpret(stackSize); MemorySegment segment12 = frame1.allocate(JAVA_INT); - Assert.assertEquals(segment12.address(), segment11.address() + JAVA_INT.byteSize()); - Assert.assertEquals(segment12.byteSize(), JAVA_INT.byteSize()); - Assert.assertEquals(segment12.scope(), frame1.scope()); + assertEquals(segment11.address() + JAVA_INT.byteSize(), segment12.address()); + assertEquals(JAVA_INT.byteSize(), segment12.byteSize()); + assertEquals(frame1.scope(), segment12.scope()); segment12.set(JAVA_INT, 0, 1); MemorySegment segment2; try (Arena frame2 = stack.pushFrame(JAVA_LONG.byteSize(), JAVA_LONG.byteAlignment())) { - Assert.assertNotEquals(frame2.scope(), frame1.scope()); + assertNotEquals(frame1.scope(), frame2.scope()); // same here, but a new scope. segment2 = frame2.allocate(JAVA_LONG); - Assert.assertEquals(segment2.address(), segment12.address() + /*segment12 size + frame 1 spare + alignment constraint*/ 3 * JAVA_INT.byteSize()); - Assert.assertEquals(segment2.byteSize(), JAVA_LONG.byteSize()); - Assert.assertEquals(segment2.scope(), frame2.scope()); + assertEquals( segment12.address() + /*segment12 size + frame 1 spare + alignment constraint*/ 3 * JAVA_INT.byteSize(), segment2.address()); + assertEquals(JAVA_LONG.byteSize(), segment2.byteSize()); + assertEquals(frame2.scope(), segment2.scope()); segment2.set(JAVA_LONG, 0, 1); // Frames must be closed in stack order. - Assert.assertThrows(IllegalStateException.class, frame1::close); + assertThrows(IllegalStateException.class, frame1::close); } // Scope is closed here, inner segments throw. - Assert.assertThrows(IllegalStateException.class, () -> segment2.get(JAVA_INT, 0)); + assertThrows(IllegalStateException.class, () -> segment2.get(JAVA_INT, 0)); // A new stack frame allocates at the same location (but different scope) as the previous did. try (Arena frame3 = stack.pushFrame(2 * JAVA_INT.byteSize(), JAVA_INT.byteAlignment())) { MemorySegment segment3 = frame3.allocate(JAVA_INT); - Assert.assertEquals(segment3.scope(), frame3.scope()); - Assert.assertEquals(segment3.address(), segment12.address() + 2 * JAVA_INT.byteSize()); + assertEquals(frame3.scope(), segment3.scope()); + assertEquals(segment12.address() + 2 * JAVA_INT.byteSize(), segment3.address()); } // Fallback arena behaves like regular stack frame. MemorySegment outOfStack; try (Arena hugeFrame = stack.pushFrame(1024, 4)) { outOfStack = hugeFrame.allocate(4); - Assert.assertEquals(outOfStack.scope(), hugeFrame.scope()); - Assert.assertTrue(outOfStack.asOverlappingSlice(stackSegment).isEmpty()); + assertEquals(hugeFrame.scope(), outOfStack.scope()); + assertTrue(outOfStack.asOverlappingSlice(stackSegment).isEmpty()); } - Assert.assertThrows(IllegalStateException.class, () -> outOfStack.get(JAVA_INT, 0)); + assertThrows(IllegalStateException.class, () -> outOfStack.get(JAVA_INT, 0)); // Outer segments are still accessible. segment11.get(JAVA_INT, 0); @@ -120,8 +121,8 @@ public void testDeepStack() throws Throwable { // Each downcall and upcall require 48 bytes of stack. // After five allocations we start falling back. MemorySegment point = recurse(10); - Assert.assertEquals(point.getAtIndex(C_DOUBLE, 0), 12.0); - Assert.assertEquals(point.getAtIndex(C_DOUBLE, 1), 11.0); - Assert.assertEquals(point.getAtIndex(C_DOUBLE, 2), 10.0); + assertEquals( 12.0, point.getAtIndex(C_DOUBLE, 0)); + assertEquals(11.0, point.getAtIndex(C_DOUBLE, 1)); + assertEquals( 10.0, point.getAtIndex(C_DOUBLE, 2)); } } diff --git a/test/jdk/java/foreign/TestBufferStackStress.java b/test/jdk/java/foreign/TestBufferStackStress.java index 757a77cc3839d..69768944fb954 100644 --- a/test/jdk/java/foreign/TestBufferStackStress.java +++ b/test/jdk/java/foreign/TestBufferStackStress.java @@ -25,12 +25,11 @@ * @test * @modules java.base/jdk.internal.foreign * @build NativeTestHelper TestBufferStackStress - * @run testng/othervm --enable-native-access=ALL-UNNAMED TestBufferStackStress + * @run junit/othervm --enable-native-access=ALL-UNNAMED TestBufferStackStress */ import jdk.internal.foreign.BufferStack; -import org.testng.Assert; -import org.testng.annotations.Test; +import org.junit.jupiter.api.Test; import java.lang.foreign.Arena; import java.lang.foreign.MemorySegment; @@ -40,6 +39,7 @@ import static java.lang.foreign.ValueLayout.*; import static java.time.temporal.ChronoUnit.SECONDS; +import static org.junit.jupiter.api.Assertions.*; public class TestBufferStackStress extends NativeTestHelper { @@ -55,7 +55,7 @@ public void stress() throws InterruptedException { // Try to assert no two vThreads get allocated the same stack space. MemorySegment segment = arena.allocate(JAVA_LONG); JAVA_LONG.varHandle().setVolatile(segment, 0L, threadId); - Assert.assertEquals(threadId, (long) JAVA_LONG.varHandle().getVolatile(segment, 0L)); + assertEquals(threadId, (long) JAVA_LONG.varHandle().getVolatile(segment, 0L)); } } Thread.yield(); // make sure the driver thread gets a chance. @@ -64,7 +64,7 @@ public void stress() throws InterruptedException { Thread.sleep(Duration.of(10, SECONDS)); Arrays.stream(vThreads).forEach( thread -> { - Assert.assertTrue(thread.isAlive()); + assertTrue(thread.isAlive()); thread.interrupt(); }); } From b1b842824b59ff14eed531b8db05bfc23ecd825d Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 30 Apr 2025 12:52:30 +0200 Subject: [PATCH 09/13] Add test coverage and add convenience methods --- .../jdk/internal/foreign/BufferStack.java | 58 ++++++- test/jdk/java/foreign/TestBufferStack.java | 159 +++++++++++++++++- .../java/foreign/TestBufferStackStress2.java | 132 +++++++++++++++ 3 files changed, 340 insertions(+), 9 deletions(-) create mode 100644 test/jdk/java/foreign/TestBufferStackStress2.java diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java index 61a5b32f6eac8..6850bf6c61c4b 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -29,9 +29,11 @@ import jdk.internal.vm.annotation.ForceInline; import java.lang.foreign.Arena; +import java.lang.foreign.MemoryLayout; import java.lang.foreign.MemorySegment; import java.lang.foreign.SegmentAllocator; import java.lang.ref.Reference; +import java.util.Objects; import java.util.concurrent.locks.ReentrantLock; /** @@ -45,8 +47,8 @@ public record BufferStack(long size, CarrierThreadLocal tl) { /** - * {@return a new Arena that tries to provided {@code size} and {@code byteAlignment} - * allocations by recycling the BufferStacks internal memory} + * {@return a new Arena that tries to provide {@code size} and {@code byteAlignment} + * allocations by recycling the BufferStacks internal memory} * * @param size to be reserved from this BufferStacks internal memory * @param byteAlignment to be used for reservation @@ -56,6 +58,32 @@ public Arena pushFrame(long size, long byteAlignment) { return tl.get().pushFrame(size, byteAlignment); } + /** + * {@return a new Arena that tries to provide {@code layout} + * allocations by recycling the BufferStacks internal memory} + * + * @param layout for which to reserve internal memory + */ + @ForceInline + public Arena pushFrame(MemoryLayout layout) { + return pushFrame(layout.byteSize(), layout.byteAlignment()); + } + + @Override + public String toString() { + return "BufferStack[" + size + "]"; + } + + @Override + public int hashCode() { + return System.identityHashCode(this); + } + + @Override + public boolean equals(Object obj) { + return this == obj; + } + private record PerThread(ReentrantLock lock, SlicingAllocator stack, Arena arena) { @ForceInline @@ -82,7 +110,7 @@ private final class Frame implements Arena { private final boolean locked; private final long parentOffset; private final long topOfStack; - private final Arena scope = Arena.ofConfined(); + private final Arena confinedArena = Arena.ofConfined(); private final SegmentAllocator frame; @SuppressWarnings("restricted") @@ -92,7 +120,7 @@ public Frame(boolean locked, long byteSize, long byteAlignment) { parentOffset = stack.currentOffset(); MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); topOfStack = stack.currentOffset(); - frame = new SlicingAllocator(frameSegment.reinterpret(scope, null)); + frame = new SlicingAllocator(frameSegment.reinterpret(confinedArena, null)); } @ForceInline @@ -105,19 +133,26 @@ private void assertOrder() { @Override @SuppressWarnings("restricted") public MemorySegment allocate(long byteSize, long byteAlignment) { + // Make sure we are on the right thread + if (((MemorySessionImpl) scope()).ownerThread() != Thread.currentThread()) { + throw MemorySessionImpl.wrongThread(); + } return frame.allocate(byteSize, byteAlignment); } + @ForceInline @Override public MemorySegment.Scope scope() { - return scope.scope(); + return confinedArena.scope(); } @ForceInline @Override public void close() { assertOrder(); - scope.close(); + // the Arena::close method is closed "early" as it checks thread + // confinement and before any mutation of the internal state takes place. + confinedArena.close(); stack.resetTo(parentOffset); if (locked) { lock.unlock(); @@ -128,6 +163,9 @@ public void close() { } public static BufferStack of(long size) { + if (size < 0) { + throw new IllegalArgumentException("Size is negative: " + size); + } return new BufferStack(size, new CarrierThreadLocal<>() { @Override protected BufferStack.PerThread initialValue() { @@ -135,4 +173,12 @@ protected BufferStack.PerThread initialValue() { } }); } + + public static BufferStack of(MemoryLayout layout) { + Objects.requireNonNull(layout); + long size = layout.byteAlignment() > 8 + ? layout.byteSize() + layout.byteAlignment() - 1 + : layout.byteSize(); + return of(size); + } } diff --git a/test/jdk/java/foreign/TestBufferStack.java b/test/jdk/java/foreign/TestBufferStack.java index 8106024898f26..42b233a30b0e9 100644 --- a/test/jdk/java/foreign/TestBufferStack.java +++ b/test/jdk/java/foreign/TestBufferStack.java @@ -23,6 +23,7 @@ /* * @test + * @library /test/lib * @modules java.base/jdk.internal.foreign * @build NativeTestHelper TestBufferStack * @run junit/othervm --enable-native-access=ALL-UNNAMED TestBufferStack @@ -33,19 +34,49 @@ import java.lang.foreign.Arena; import java.lang.foreign.FunctionDescriptor; +import java.lang.foreign.Linker; import java.lang.foreign.MemoryLayout; import java.lang.foreign.MemorySegment; import java.lang.foreign.SegmentAllocator; import java.lang.invoke.MethodHandle; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; + +import jdk.test.lib.thread.VThreadRunner; import static java.lang.foreign.MemoryLayout.structLayout; import static java.lang.foreign.ValueLayout.*; import static org.junit.jupiter.api.Assertions.*; -public class TestBufferStack extends NativeTestHelper { +final class TestBufferStack extends NativeTestHelper { + + private static final long POOL_SIZE = 64; + private static final long SMALL_ALLOC_SIZE = 8; + + @Test + void invariants() { + var ex = assertThrows(IllegalArgumentException.class, () -> BufferStack.of(-1)); + assertEquals("Size is negative: -1", ex.getMessage()); + assertThrows(NullPointerException.class, () -> BufferStack.of(null)); + + BufferStack stack = BufferStack.of(POOL_SIZE); + assertThrows(IllegalArgumentException.class, () -> stack.pushFrame(-1, 8)); + assertThrows(IllegalArgumentException.class, () -> stack.pushFrame(SMALL_ALLOC_SIZE, -1)); + + try (var arena = stack.pushFrame(SMALL_ALLOC_SIZE, 1)) { + assertThrows(IllegalArgumentException.class, () -> arena.allocate(-1)); + assertThrows(IllegalArgumentException.class, () -> arena.allocate(4, -1)); + } + } + + @Test + void invariantsVt() { + VThreadRunner.run(this::invariants); + } @Test - public void testScopedAllocation() { + void testScopedAllocation() { int stackSize = 128; BufferStack stack = BufferStack.of(stackSize); MemorySegment stackSegment; @@ -100,6 +131,11 @@ public void testScopedAllocation() { } } + @Test + void testScopedAllocationVt() { + VThreadRunner.run(this::testScopedAllocation); + } + static { System.loadLibrary("TestBufferStack"); } @@ -117,7 +153,7 @@ public static MemorySegment recurse(int depth) { } @Test - public void testDeepStack() throws Throwable { + void testDeepStack() { // Each downcall and upcall require 48 bytes of stack. // After five allocations we start falling back. MemorySegment point = recurse(10); @@ -125,4 +161,121 @@ public void testDeepStack() throws Throwable { assertEquals(11.0, point.getAtIndex(C_DOUBLE, 1)); assertEquals( 10.0, point.getAtIndex(C_DOUBLE, 2)); } + + @Test + void testDeepStackVt() { + VThreadRunner.run(this::testDeepStack); + } + + @Test + void equals() { + var first = BufferStack.of(POOL_SIZE); + var second = BufferStack.of(POOL_SIZE); + assertNotEquals(first, second); + assertEquals(first, first); + } + + @Test + void allocationSameAsPoolSize() { + MemoryLayout twoInts = MemoryLayout.sequenceLayout(2, JAVA_INT); + var pool = BufferStack.of(POOL_SIZE); + long firstAddress; + try (var arena = pool.pushFrame(JAVA_INT)) { + var segment = arena.allocate(JAVA_INT); + firstAddress = segment.address(); + } + for (int i = 0; i < 10; i++) { + try (var arena = pool.pushFrame(twoInts)) { + var segment = arena.allocate(JAVA_INT); + assertEquals(firstAddress, segment.address()); + var segmentTwo = arena.allocate(JAVA_INT); + assertNotEquals(firstAddress, segmentTwo.address()); + } + } + } + + @Test + void allocationSameAsPoolSizeVt() { + VThreadRunner.run(this::allocationSameAsPoolSize); + } + + @Test + void allocationCaptureStateLayout() { + var layout = Linker.Option.captureStateLayout(); + var pool = BufferStack.of(layout); + long firstAddress; + try (var arena = pool.pushFrame(layout)) { + var segment = arena.allocate(layout); + firstAddress = segment.address(); + } + for (int i = 0; i < 10; i++) { + try (var arena = pool.pushFrame(layout)) { + var segment = arena.allocate(layout); + assertEquals(firstAddress, segment.address()); + // Questionable exception type + assertThrows(IndexOutOfBoundsException.class, () -> arena.allocate(layout)); + } + } + } + + @Test + void allocateConfinement() { + var pool = BufferStack.of(POOL_SIZE); + Consumer allocateAction = arena -> + assertThrows(WrongThreadException.class, () -> { + CompletableFuture future = CompletableFuture.supplyAsync(() -> pool.pushFrame(SMALL_ALLOC_SIZE, 1)); + var otherThreadArena = future.get(); + otherThreadArena.allocate(SMALL_ALLOC_SIZE); + // Intentionally do not close the otherThreadArena here. + }); + doInTwoStackedArenas(pool, allocateAction, allocateAction); + } + + @Test + void allocateConfinementVt() { + VThreadRunner.run(this::allocateConfinement); + } + + @Test + void closeConfinement() { + var pool = BufferStack.of(POOL_SIZE); + Consumer closeAction = arena -> { + // 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.pushFrame(SMALL_ALLOC_SIZE, 1)); + }); + try { + thread.join(); + } catch (InterruptedException ie) { + fail(ie); + } + assertThrows(WrongThreadException.class, otherThreadArena.get()::close); + }; + doInTwoStackedArenas(pool, closeAction, closeAction); + } + + @Test + void closeConfinementVt() { + VThreadRunner.run(this::closeConfinement); + } + + @Test + void toStringTest() { + BufferStack stack = BufferStack.of(POOL_SIZE); + assertEquals("BufferStack[" + POOL_SIZE + "]", stack.toString()); + } + + static void doInTwoStackedArenas(BufferStack pool, + Consumer firstAction, + Consumer secondAction) { + try (var firstArena = pool.pushFrame(SMALL_ALLOC_SIZE, 1)) { + firstAction.accept(firstArena); + try (var secondArena = pool.pushFrame(SMALL_ALLOC_SIZE, 1)) { + secondAction.accept(secondArena); + } + } + } + } diff --git a/test/jdk/java/foreign/TestBufferStackStress2.java b/test/jdk/java/foreign/TestBufferStackStress2.java new file mode 100644 index 0000000000000..8fbb8406bb57a --- /dev/null +++ b/test/jdk/java/foreign/TestBufferStackStress2.java @@ -0,0 +1,132 @@ +/* + * 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 + * @library /test/lib + * @modules java.base/jdk.internal.foreign + * @build NativeTestHelper TestBufferStackStress2 + * @run junit/othervm --enable-native-access=ALL-UNNAMED TestBufferStackStress2 + */ + +import jdk.internal.foreign.BufferStack; +import org.junit.jupiter.api.Test; + +import java.io.FileDescriptor; +import java.lang.foreign.Arena; +import java.lang.foreign.MemorySegment; +import java.lang.foreign.ValueLayout; +import java.time.Duration; +import java.time.temporal.ChronoUnit; +import java.util.concurrent.Executors; +import java.util.concurrent.ForkJoinWorkerThread; +import java.util.concurrent.atomic.AtomicBoolean; + +final class TestBufferStackStress2 extends NativeTestHelper { + + private static final long POOL_SIZE = 64; + private static final long SMALL_ALLOC_SIZE = 8; + + /** + * The objective with this test is to test the case when a virtual thread VT0 is + * mounted on a carrier thread CT0; VT0 is then suspended; The pool of carrier threads + * are then contracted; VT0 is then remounted on another carrier thread C1. VT0 runs + * for a while when there is a lot of GC activity. + * In other words, we are trying to establish that there is no use-after-free and that + * the original arena, from which reusable segments are initially allocated from, is + * not closed underneath. + *

+ * Unfortunately, this test takes about 30 seconds as that is the time it takes for + * the pool of carrier threads to be contracted. + */ + @Test + void movingVirtualThreadWithGc() throws InterruptedException { + final long begin = System.nanoTime(); + var pool = BufferStack.of(POOL_SIZE); + + System.setProperty("jdk.virtualThreadScheduler.parallelism", "1"); + + var done = new AtomicBoolean(); + var quiescent = new Object(); + var executor = Executors.newVirtualThreadPerTaskExecutor(); + + executor.submit(() -> { + while (!done.get()) { + FileDescriptor.out.sync(); + } + return null; + }); + + executor.submit(() -> { + System.out.println(duration(begin) + "ALLOCATING = " + Thread.currentThread()); + try (Arena arena = pool.pushFrame(SMALL_ALLOC_SIZE, 1)) { + MemorySegment segment = arena.allocate(SMALL_ALLOC_SIZE); + done.set(true); + synchronized (quiescent) { + try { + quiescent.wait(); + } catch (Throwable ex) { + throw new AssertionError(ex); + } + } + System.out.println(duration(begin) + "ACCESSING SEGMENT"); + + for (int i = 0; i < 100_000; i++) { + if (i % 100 == 0) { + System.gc(); + } + segment.get(ValueLayout.JAVA_BYTE, i); + } + System.out.println(duration(begin) + "DONE ACCESSING SEGMENT"); + } + System.out.println(duration(begin) + "VT DONE"); + }); + + long count; + do { + Thread.sleep(1000); + count = Thread.getAllStackTraces().keySet().stream() + .filter(t -> t instanceof ForkJoinWorkerThread) + .count(); + } while (count > 0); + + System.out.println(duration(begin) + "FJP HAS CONTRACTED"); + + synchronized (quiescent) { + quiescent.notify(); + } + + System.out.println(duration(begin) + "CLOSING EXECUTOR"); + executor.close(); + System.out.println(duration(begin) + "EXECUTOR CLOSED"); + } + + private static String duration(Long begin) { + var duration = Duration.of(System.nanoTime() - begin, ChronoUnit.NANOS); + long seconds = duration.toSeconds(); + int nanos = duration.toNanosPart(); + return (Thread.currentThread().isVirtual() ? "VT: " : "PT: ") + + String.format("%3d:%09d ", seconds, nanos); + } + +} From f976328507384311b6b117a224986ce38e26aee5 Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 30 Apr 2025 17:27:30 +0200 Subject: [PATCH 10/13] Use utility method rather than custom calculation --- .../share/classes/jdk/internal/foreign/BufferStack.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java index 6850bf6c61c4b..a87af46958086 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -177,7 +177,7 @@ protected BufferStack.PerThread initialValue() { public static BufferStack of(MemoryLayout layout) { Objects.requireNonNull(layout); long size = layout.byteAlignment() > 8 - ? layout.byteSize() + layout.byteAlignment() - 1 + ? Utils.alignUp(layout.byteSize(), layout.byteAlignment()) : layout.byteSize(); return of(size); } From 22c384f9b0feee80e315b360b7760416cea9a2bd Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Wed, 30 Apr 2025 17:29:41 +0200 Subject: [PATCH 11/13] Improve on comments --- .../classes/jdk/internal/foreign/BufferStack.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java index a87af46958086..aa103a9625468 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -36,6 +36,8 @@ import java.util.Objects; import java.util.concurrent.locks.ReentrantLock; +import static java.lang.foreign.ValueLayout.JAVA_LONG; + /** * A buffer stack that allows efficient reuse of memory segments. This is useful in cases * where temporary memory is needed. @@ -150,8 +152,9 @@ public MemorySegment.Scope scope() { @Override public void close() { assertOrder(); - // the Arena::close method is closed "early" as it checks thread - // confinement and before any mutation of the internal state takes place. + // the Arena::close method is called "early" as it checks thread + // confinement and crucially before any mutation of the internal + // state takes place. confinedArena.close(); stack.resetTo(parentOffset); if (locked) { @@ -176,7 +179,8 @@ protected BufferStack.PerThread initialValue() { public static BufferStack of(MemoryLayout layout) { Objects.requireNonNull(layout); - long size = layout.byteAlignment() > 8 + // Allocations are always at least aligned with the largest Java type (e.g., long) + long size = layout.byteAlignment() > JAVA_LONG.byteSize() ? Utils.alignUp(layout.byteSize(), layout.byteAlignment()) : layout.byteSize(); return of(size); From dcce2e948efe9bf1900c7c7216e6341e914c109f Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Fri, 2 May 2025 11:41:04 +0200 Subject: [PATCH 12/13] Address comments --- .../jdk/internal/foreign/BufferStack.java | 112 ++++++++++-------- .../jdk/internal/foreign/abi/SharedUtils.java | 2 +- test/jdk/java/foreign/TestBufferStack.java | 74 ++++++------ .../java/foreign/TestBufferStackStress.java | 6 +- .../java/foreign/TestBufferStackStress2.java | 14 ++- .../lang/foreign/CallOverheadByValue.java | 8 +- 6 files changed, 122 insertions(+), 94 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java index aa103a9625468..dd7886149d02b 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -33,36 +33,58 @@ import java.lang.foreign.MemorySegment; import java.lang.foreign.SegmentAllocator; import java.lang.ref.Reference; -import java.util.Objects; import java.util.concurrent.locks.ReentrantLock; - -import static java.lang.foreign.ValueLayout.JAVA_LONG; +import java.util.function.Consumer; /** * A buffer stack that allows efficient reuse of memory segments. This is useful in cases * where temporary memory is needed. *

- * Use the factory {@link #of(long)} to create new instances of this class. + * Use the factories {@code BufferStack.of(...)} to create new instances of this class. *

* Note: The reused segments are neither zeroed out before nor after re-use. */ -public record BufferStack(long size, CarrierThreadLocal tl) { +public final class BufferStack { + + private final long byteSize; + private final CarrierThreadLocal tl; + + public BufferStack(long byteSize, long byteAlignment) { + this.byteSize = byteSize; + this.tl = new CarrierThreadLocal<>() { + @Override + protected BufferStack.PerThread initialValue() { + return BufferStack.PerThread.of(byteSize, byteAlignment); + } + }; + } /** - * {@return a new Arena that tries to provide {@code size} and {@code byteAlignment} - * allocations by recycling the BufferStacks internal memory} + * {@return a new Arena that tries to provide {@code byteSize} and {@code byteAlignment} + * allocations by recycling the BufferStack's internal memory} * - * @param size to be reserved from this BufferStacks internal memory + * @param byteSize to be reserved from this BufferStack's internal memory * @param byteAlignment to be used for reservation */ @ForceInline - public Arena pushFrame(long size, long byteAlignment) { - return tl.get().pushFrame(size, byteAlignment); + public Arena pushFrame(long byteSize, long byteAlignment) { + return tl.get().pushFrame(byteSize, byteAlignment); + } + + /** + * {@return a new Arena that tries to provide {@code byteSize} + * allocations by recycling the BufferStack's internal memory} + * + * @param byteSize to be reserved from this BufferStack's internal memory + */ + @ForceInline + public Arena pushFrame(long byteSize) { + return pushFrame(byteSize, 1); } /** * {@return a new Arena that tries to provide {@code layout} - * allocations by recycling the BufferStacks internal memory} + * allocations by recycling the BufferStack's internal memory} * * @param layout for which to reserve internal memory */ @@ -73,20 +95,10 @@ public Arena pushFrame(MemoryLayout layout) { @Override public String toString() { - return "BufferStack[" + size + "]"; + return "BufferStack[" + byteSize + "]"; } - @Override - public int hashCode() { - return System.identityHashCode(this); - } - - @Override - public boolean equals(Object obj) { - return this == obj; - } - - private record PerThread(ReentrantLock lock, SlicingAllocator stack, Arena arena) { + private record PerThread(ReentrantLock lock, Arena arena, SlicingAllocator stack) { @ForceInline public Arena pushFrame(long size, long byteAlignment) { @@ -102,10 +114,11 @@ public Arena pushFrame(long size, long byteAlignment) { return new Frame(needsLock, size, byteAlignment); } - static PerThread of(long size) { + static PerThread of(long byteSize, long byteAlignment) { final Arena arena = Arena.ofAuto(); - final SlicingAllocator stack = new SlicingAllocator(arena.allocate(size)); - return new PerThread(new ReentrantLock(), stack, arena); + return new PerThread(new ReentrantLock(), + arena, + new SlicingAllocator(arena.allocate(byteSize, byteAlignment))); } private final class Frame implements Arena { @@ -122,7 +135,17 @@ public Frame(boolean locked, long byteSize, long byteAlignment) { parentOffset = stack.currentOffset(); MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); topOfStack = stack.currentOffset(); - frame = new SlicingAllocator(frameSegment.reinterpret(confinedArena, null)); + // The cleanup action will keep the original automatic `arena` (from which + // the reusable segment is first allocated) alive even if this Frame + // becomes unreachable but there are reachable segments still alive. + frame = new SlicingAllocator(frameSegment.reinterpret(confinedArena, new CleanupAction(arena))); + } + + record CleanupAction(Arena arena) implements Consumer { + @Override + public void accept(MemorySegment memorySegment) { + Reference.reachabilityFence(arena); + } } @ForceInline @@ -135,10 +158,8 @@ private void assertOrder() { @Override @SuppressWarnings("restricted") public MemorySegment allocate(long byteSize, long byteAlignment) { - // Make sure we are on the right thread - if (((MemorySessionImpl) scope()).ownerThread() != Thread.currentThread()) { - throw MemorySessionImpl.wrongThread(); - } + // Make sure we are on the right thread and not closed + MemorySessionImpl.toMemorySession(confinedArena).checkValidState(); return frame.allocate(byteSize, byteAlignment); } @@ -160,29 +181,26 @@ public void close() { if (locked) { lock.unlock(); } - Reference.reachabilityFence(arena); } } } - public static BufferStack of(long size) { - if (size < 0) { - throw new IllegalArgumentException("Size is negative: " + size); + public static BufferStack of(long byteSize, long byteAlignment) { + if (byteSize < 0) { + throw new IllegalArgumentException("Negative byteSize: " + byteSize); } - return new BufferStack(size, new CarrierThreadLocal<>() { - @Override - protected BufferStack.PerThread initialValue() { - return BufferStack.PerThread.of(size); - } - }); + if (byteAlignment < 0) { + throw new IllegalArgumentException("Negative byteAlignment: " + byteAlignment); + } + return new BufferStack(byteSize, byteAlignment); + } + + public static BufferStack of(long byteSize) { + return new BufferStack(byteSize, 1); } public static BufferStack of(MemoryLayout layout) { - Objects.requireNonNull(layout); - // Allocations are always at least aligned with the largest Java type (e.g., long) - long size = layout.byteAlignment() > JAVA_LONG.byteSize() - ? Utils.alignUp(layout.byteSize(), layout.byteAlignment()) - : layout.byteSize(); - return of(size); + // Implicit null check + return of(layout.byteSize(), layout.byteAlignment()); } } diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java b/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java index 0204906961661..37200598d5b07 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java +++ b/src/java.base/share/classes/jdk/internal/foreign/abi/SharedUtils.java @@ -391,7 +391,7 @@ static long pickChunkOffset(long chunkOffset, long byteWidth, int chunkWidth) { } private static final int LINKER_STACK_SIZE = Integer.getInteger("jdk.internal.foreign.LINKER_STACK_SIZE", 256); - private static final BufferStack LINKER_STACK = BufferStack.of(LINKER_STACK_SIZE); + private static final BufferStack LINKER_STACK = BufferStack.of(LINKER_STACK_SIZE, 1); @ForceInline public static Arena newBoundedArena(long size) { diff --git a/test/jdk/java/foreign/TestBufferStack.java b/test/jdk/java/foreign/TestBufferStack.java index 42b233a30b0e9..af5945bb58601 100644 --- a/test/jdk/java/foreign/TestBufferStack.java +++ b/test/jdk/java/foreign/TestBufferStack.java @@ -56,11 +56,13 @@ final class TestBufferStack extends NativeTestHelper { @Test void invariants() { - var ex = assertThrows(IllegalArgumentException.class, () -> BufferStack.of(-1)); - assertEquals("Size is negative: -1", ex.getMessage()); + var exBS = assertThrows(IllegalArgumentException.class, () -> BufferStack.of(-1, 1)); + assertEquals("Negative byteSize: -1", exBS.getMessage()); + var exBA = assertThrows(IllegalArgumentException.class, () -> BufferStack.of(1, -1)); + assertEquals("Negative byteAlignment: -1", exBA.getMessage()); assertThrows(NullPointerException.class, () -> BufferStack.of(null)); - BufferStack stack = BufferStack.of(POOL_SIZE); + BufferStack stack = newBufferStack(); assertThrows(IllegalArgumentException.class, () -> stack.pushFrame(-1, 8)); assertThrows(IllegalArgumentException.class, () -> stack.pushFrame(SMALL_ALLOC_SIZE, -1)); @@ -78,15 +80,13 @@ void invariantsVt() { @Test void testScopedAllocation() { int stackSize = 128; - BufferStack stack = BufferStack.of(stackSize); - MemorySegment stackSegment; + BufferStack stack = newBufferStack(); try (Arena frame1 = stack.pushFrame(3 * JAVA_INT.byteSize(), JAVA_INT.byteAlignment())) { // Segments have expected sizes and are accessible and allocated consecutively in the same scope. MemorySegment segment11 = frame1.allocate(JAVA_INT); assertEquals(frame1.scope(), segment11.scope()); assertEquals(JAVA_INT.byteSize(), segment11.byteSize()); segment11.set(JAVA_INT, 0, 1); - stackSegment = segment11.reinterpret(stackSize); MemorySegment segment12 = frame1.allocate(JAVA_INT); assertEquals(segment11.address() + JAVA_INT.byteSize(), segment12.address()); @@ -121,7 +121,7 @@ void testScopedAllocation() { try (Arena hugeFrame = stack.pushFrame(1024, 4)) { outOfStack = hugeFrame.allocate(4); assertEquals(hugeFrame.scope(), outOfStack.scope()); - assertTrue(outOfStack.asOverlappingSlice(stackSegment).isEmpty()); + assertTrue(outOfStack.asOverlappingSlice(segment11).isEmpty()); } assertThrows(IllegalStateException.class, () -> outOfStack.get(JAVA_INT, 0)); @@ -169,8 +169,8 @@ void testDeepStackVt() { @Test void equals() { - var first = BufferStack.of(POOL_SIZE); - var second = BufferStack.of(POOL_SIZE); + var first = newBufferStack(); + var second = newBufferStack(); assertNotEquals(first, second); assertEquals(first, first); } @@ -178,7 +178,7 @@ void equals() { @Test void allocationSameAsPoolSize() { MemoryLayout twoInts = MemoryLayout.sequenceLayout(2, JAVA_INT); - var pool = BufferStack.of(POOL_SIZE); + var pool = newBufferStack(); long firstAddress; try (var arena = pool.pushFrame(JAVA_INT)) { var segment = arena.allocate(JAVA_INT); @@ -189,38 +189,20 @@ void allocationSameAsPoolSize() { var segment = arena.allocate(JAVA_INT); assertEquals(firstAddress, segment.address()); var segmentTwo = arena.allocate(JAVA_INT); - assertNotEquals(firstAddress, segmentTwo.address()); + assertEquals(firstAddress + JAVA_INT.byteSize(), segmentTwo.address()); + // Questionable exception type + assertThrows(IndexOutOfBoundsException.class, () -> arena.allocate(JAVA_INT)); } } } - @Test void allocationSameAsPoolSizeVt() { VThreadRunner.run(this::allocationSameAsPoolSize); } - @Test - void allocationCaptureStateLayout() { - var layout = Linker.Option.captureStateLayout(); - var pool = BufferStack.of(layout); - long firstAddress; - try (var arena = pool.pushFrame(layout)) { - var segment = arena.allocate(layout); - firstAddress = segment.address(); - } - for (int i = 0; i < 10; i++) { - try (var arena = pool.pushFrame(layout)) { - var segment = arena.allocate(layout); - assertEquals(firstAddress, segment.address()); - // Questionable exception type - assertThrows(IndexOutOfBoundsException.class, () -> arena.allocate(layout)); - } - } - } - @Test void allocateConfinement() { - var pool = BufferStack.of(POOL_SIZE); + var pool = newBufferStack(); Consumer allocateAction = arena -> assertThrows(WrongThreadException.class, () -> { CompletableFuture future = CompletableFuture.supplyAsync(() -> pool.pushFrame(SMALL_ALLOC_SIZE, 1)); @@ -238,7 +220,7 @@ void allocateConfinementVt() { @Test void closeConfinement() { - var pool = BufferStack.of(POOL_SIZE); + var pool = newBufferStack(); Consumer closeAction = arena -> { // Do not use CompletableFuture here as it might accidentally run on the // same carrier thread as a virtual thread. @@ -263,10 +245,29 @@ void closeConfinementVt() { @Test void toStringTest() { - BufferStack stack = BufferStack.of(POOL_SIZE); + BufferStack stack = newBufferStack(); assertEquals("BufferStack[" + POOL_SIZE + "]", stack.toString()); } + @Test + void allocBounds() { + BufferStack stack = newBufferStack(); + try (var arena = stack.pushFrame(SMALL_ALLOC_SIZE, 1)) { + assertThrows(IllegalArgumentException.class, () -> arena.allocate(-1)); + assertDoesNotThrow(() -> arena.allocate(SMALL_ALLOC_SIZE)); + assertThrows(IndexOutOfBoundsException.class, () -> arena.allocate(SMALL_ALLOC_SIZE + 1)); + } + } + + @Test + void accessBounds() { + BufferStack stack = newBufferStack(); + try (var arena = stack.pushFrame(SMALL_ALLOC_SIZE, 1)) { + var segment = arena.allocate(SMALL_ALLOC_SIZE); + assertThrows(IndexOutOfBoundsException.class, () -> segment.get(JAVA_BYTE, SMALL_ALLOC_SIZE)); + } + } + static void doInTwoStackedArenas(BufferStack pool, Consumer firstAction, Consumer secondAction) { @@ -278,4 +279,9 @@ static void doInTwoStackedArenas(BufferStack pool, } } + private static BufferStack newBufferStack() { + return BufferStack.of(POOL_SIZE, 1); + } + + } diff --git a/test/jdk/java/foreign/TestBufferStackStress.java b/test/jdk/java/foreign/TestBufferStackStress.java index 69768944fb954..0ec46311a6f17 100644 --- a/test/jdk/java/foreign/TestBufferStackStress.java +++ b/test/jdk/java/foreign/TestBufferStackStress.java @@ -25,7 +25,7 @@ * @test * @modules java.base/jdk.internal.foreign * @build NativeTestHelper TestBufferStackStress - * @run junit/othervm --enable-native-access=ALL-UNNAMED TestBufferStackStress + * @run junit TestBufferStackStress */ import jdk.internal.foreign.BufferStack; @@ -41,11 +41,11 @@ import static java.time.temporal.ChronoUnit.SECONDS; import static org.junit.jupiter.api.Assertions.*; -public class TestBufferStackStress extends NativeTestHelper { +public class TestBufferStackStress { @Test public void stress() throws InterruptedException { - BufferStack stack = BufferStack.of(256); + BufferStack stack = BufferStack.of(256, 1); Thread[] vThreads = IntStream.range(0, 1024).mapToObj(_ -> Thread.ofVirtual().start(() -> { long threadId = Thread.currentThread().threadId(); diff --git a/test/jdk/java/foreign/TestBufferStackStress2.java b/test/jdk/java/foreign/TestBufferStackStress2.java index 8fbb8406bb57a..4b02f4691fb8a 100644 --- a/test/jdk/java/foreign/TestBufferStackStress2.java +++ b/test/jdk/java/foreign/TestBufferStackStress2.java @@ -23,10 +23,9 @@ /* * @test - * @library /test/lib * @modules java.base/jdk.internal.foreign * @build NativeTestHelper TestBufferStackStress2 - * @run junit/othervm --enable-native-access=ALL-UNNAMED TestBufferStackStress2 + * @run junit TestBufferStackStress2 */ import jdk.internal.foreign.BufferStack; @@ -42,7 +41,9 @@ import java.util.concurrent.ForkJoinWorkerThread; import java.util.concurrent.atomic.AtomicBoolean; -final class TestBufferStackStress2 extends NativeTestHelper { +import static org.junit.jupiter.api.Assertions.*; + +final class TestBufferStackStress2 { private static final long POOL_SIZE = 64; private static final long SMALL_ALLOC_SIZE = 8; @@ -62,11 +63,12 @@ final class TestBufferStackStress2 extends NativeTestHelper { @Test void movingVirtualThreadWithGc() throws InterruptedException { final long begin = System.nanoTime(); - var pool = BufferStack.of(POOL_SIZE); + var pool = BufferStack.of(POOL_SIZE, 1); System.setProperty("jdk.virtualThreadScheduler.parallelism", "1"); var done = new AtomicBoolean(); + var completed = new AtomicBoolean(); var quiescent = new Object(); var executor = Executors.newVirtualThreadPerTaskExecutor(); @@ -95,11 +97,12 @@ void movingVirtualThreadWithGc() throws InterruptedException { if (i % 100 == 0) { System.gc(); } - segment.get(ValueLayout.JAVA_BYTE, i); + segment.get(ValueLayout.JAVA_BYTE, i % SMALL_ALLOC_SIZE); } System.out.println(duration(begin) + "DONE ACCESSING SEGMENT"); } System.out.println(duration(begin) + "VT DONE"); + completed.set(true); }); long count; @@ -119,6 +122,7 @@ void movingVirtualThreadWithGc() throws InterruptedException { System.out.println(duration(begin) + "CLOSING EXECUTOR"); executor.close(); System.out.println(duration(begin) + "EXECUTOR CLOSED"); + assertTrue(completed.get(), "The VT did not complete properly"); } private static String duration(Long begin) { diff --git a/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java b/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java index ab490a2f3ebcd..18ddcda495a8c 100644 --- a/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java +++ b/test/micro/org/openjdk/bench/java/lang/foreign/CallOverheadByValue.java @@ -73,8 +73,9 @@ public class CallOverheadByValue { ); } - Arena arena = Arena.ofConfined(); - MemorySegment point = arena.allocate(POINT_LAYOUT); + private static final Arena arena = Arena.ofConfined(); + private static final MemorySegment point = arena.allocate(POINT_LAYOUT); + private static final SegmentAllocator BY_VALUE_ALLOCATOR = (SegmentAllocator) (_, _) -> point; @TearDown public void tearDown() { @@ -84,8 +85,7 @@ public void tearDown() { @Benchmark public void byValue() throws Throwable { // point = unit(); - MemorySegment unused = (MemorySegment) MH_UNIT_BY_VALUE.invokeExact( - (SegmentAllocator) (_, _) -> point); + MemorySegment unused = (MemorySegment) MH_UNIT_BY_VALUE.invokeExact(BY_VALUE_ALLOCATOR); } @Benchmark From c819553c3effa31298582d97dc39a4d89412aa0b Mon Sep 17 00:00:00 2001 From: Per Minborg Date: Fri, 2 May 2025 13:10:30 +0200 Subject: [PATCH 13/13] Cleanup and only create CleanupAction once per thread --- .../jdk/internal/foreign/BufferStack.java | 40 +++++++++++-------- test/jdk/java/foreign/TestBufferStack.java | 3 +- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java index dd7886149d02b..dbf21601c53ac 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java +++ b/src/java.base/share/classes/jdk/internal/foreign/BufferStack.java @@ -47,10 +47,12 @@ public final class BufferStack { private final long byteSize; + private final long byteAlignment; private final CarrierThreadLocal tl; - public BufferStack(long byteSize, long byteAlignment) { + private BufferStack(long byteSize, long byteAlignment) { this.byteSize = byteSize; + this.byteAlignment = byteAlignment; this.tl = new CarrierThreadLocal<>() { @Override protected BufferStack.PerThread initialValue() { @@ -95,10 +97,13 @@ public Arena pushFrame(MemoryLayout layout) { @Override public String toString() { - return "BufferStack[" + byteSize + "]"; + return "BufferStack[byteSize=" + byteSize + ", byteAlignment=" + byteAlignment + "]"; } - private record PerThread(ReentrantLock lock, Arena arena, SlicingAllocator stack) { + private record PerThread(ReentrantLock lock, + Arena arena, + SlicingAllocator stack, + CleanupAction cleanupAction) { @ForceInline public Arena pushFrame(long size, long byteAlignment) { @@ -118,34 +123,37 @@ static PerThread of(long byteSize, long byteAlignment) { final Arena arena = Arena.ofAuto(); return new PerThread(new ReentrantLock(), arena, - new SlicingAllocator(arena.allocate(byteSize, byteAlignment))); + new SlicingAllocator(arena.allocate(byteSize, byteAlignment)), + new CleanupAction(arena)); + } + + private record CleanupAction(Arena arena) implements Consumer { + @Override + public void accept(MemorySegment memorySegment) { + Reference.reachabilityFence(arena); + } } private final class Frame implements Arena { + private final boolean locked; private final long parentOffset; private final long topOfStack; - private final Arena confinedArena = Arena.ofConfined(); + private final Arena confinedArena; private final SegmentAllocator frame; @SuppressWarnings("restricted") @ForceInline public Frame(boolean locked, long byteSize, long byteAlignment) { this.locked = locked; - parentOffset = stack.currentOffset(); - MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); - topOfStack = stack.currentOffset(); + this.parentOffset = stack.currentOffset(); + final MemorySegment frameSegment = stack.allocate(byteSize, byteAlignment); + this.topOfStack = stack.currentOffset(); + this.confinedArena = Arena.ofConfined(); // The cleanup action will keep the original automatic `arena` (from which // the reusable segment is first allocated) alive even if this Frame // becomes unreachable but there are reachable segments still alive. - frame = new SlicingAllocator(frameSegment.reinterpret(confinedArena, new CleanupAction(arena))); - } - - record CleanupAction(Arena arena) implements Consumer { - @Override - public void accept(MemorySegment memorySegment) { - Reference.reachabilityFence(arena); - } + this.frame = new SlicingAllocator(frameSegment.reinterpret(confinedArena, cleanupAction)); } @ForceInline diff --git a/test/jdk/java/foreign/TestBufferStack.java b/test/jdk/java/foreign/TestBufferStack.java index af5945bb58601..f7bf67bfb50f0 100644 --- a/test/jdk/java/foreign/TestBufferStack.java +++ b/test/jdk/java/foreign/TestBufferStack.java @@ -34,7 +34,6 @@ import java.lang.foreign.Arena; import java.lang.foreign.FunctionDescriptor; -import java.lang.foreign.Linker; import java.lang.foreign.MemoryLayout; import java.lang.foreign.MemorySegment; import java.lang.foreign.SegmentAllocator; @@ -246,7 +245,7 @@ void closeConfinementVt() { @Test void toStringTest() { BufferStack stack = newBufferStack(); - assertEquals("BufferStack[" + POOL_SIZE + "]", stack.toString()); + assertEquals("BufferStack[byteSize=" + POOL_SIZE + ", byteAlignment=1]", stack.toString()); } @Test