From ab45a7b053e315632988a2d14da707f96befb9c4 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Wed, 7 Apr 2021 16:19:35 +0200 Subject: [PATCH] Fix (some) failing tests Also introduce test sampling, so when the BufferTest is running from an IDE, only 3% of the tests will actually run. The Maven build runs all tests. --- pom.xml | 3 ++ .../netty/buffer/api/memseg/MemSegBuffer.java | 11 ++++- .../memseg/NativeMemorySegmentManager.java | 4 +- .../java/io/netty/buffer/api/BufferTest.java | 44 ++++++++++++++----- 4 files changed, 48 insertions(+), 14 deletions(-) diff --git a/pom.xml b/pom.xml index 6b7fe3e..e5fb8c5 100644 --- a/pom.xml +++ b/pom.xml @@ -173,6 +173,9 @@ ${argLine.common} ${argLine.printGC} --add-modules jdk.incubator.foreign false + + nosample + diff --git a/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java b/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java index b99b81c..842e48a 100644 --- a/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java +++ b/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java @@ -60,12 +60,19 @@ class MemSegBuffer extends RcSupport implements Buffer, Re static { try (ResourceScope scope = ResourceScope.newSharedScope()) { - CLOSED_SEGMENT = MemorySegment.allocateNative(1, scope); + // We are not allowed to allocate a zero-sized native buffer, but we *can* take a zero-sized slice from it. + // We need the CLOSED_SEGMENT to have a size of zero, because we'll use its size for bounds checks after + // the buffer is closed. + MemorySegment segment = MemorySegment.allocateNative(1, scope); + CLOSED_SEGMENT = segment.asSlice(0, 0); } SEGMENT_CLOSE = new Drop() { @Override public void drop(MemSegBuffer buf) { - buf.base.scope().close(); + ResourceScope scope = buf.base.scope(); + if (!scope.isImplicit()) { + scope.close(); + } } @Override diff --git a/src/main/java/io/netty/buffer/api/memseg/NativeMemorySegmentManager.java b/src/main/java/io/netty/buffer/api/memseg/NativeMemorySegmentManager.java index b49395e..98fe021 100644 --- a/src/main/java/io/netty/buffer/api/memseg/NativeMemorySegmentManager.java +++ b/src/main/java/io/netty/buffer/api/memseg/NativeMemorySegmentManager.java @@ -22,6 +22,8 @@ import java.lang.ref.Cleaner; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.LongAdder; +import static jdk.incubator.foreign.ResourceScope.newSharedScope; + public class NativeMemorySegmentManager extends AbstractMemorySegmentManager { public static final LongAdder MEM_USAGE_NATIVE = new LongAdder(); private static final ConcurrentHashMap CLEANUP_ACTIONS = new ConcurrentHashMap<>(); @@ -37,7 +39,7 @@ public class NativeMemorySegmentManager extends AbstractMemorySegmentManager { @Override protected MemorySegment createSegment(long size, Cleaner cleaner) { - final ResourceScope scope = ResourceScope.newSharedScope(cleaner); + final ResourceScope scope = cleaner == null ? newSharedScope() : newSharedScope(cleaner); scope.addOnClose(getCleanupAction(size)); var segment = MemorySegment.allocateNative(size, scope); MEM_USAGE_NATIVE.add(size); diff --git a/src/test/java/io/netty/buffer/api/BufferTest.java b/src/test/java/io/netty/buffer/api/BufferTest.java index f2a9c43..e3ccdbf 100644 --- a/src/test/java/io/netty/buffer/api/BufferTest.java +++ b/src/test/java/io/netty/buffer/api/BufferTest.java @@ -31,10 +31,14 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.ReadOnlyBufferException; import java.text.ParseException; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.time.temporal.TemporalUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedList; import java.util.List; +import java.util.SplittableRandom; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -43,6 +47,7 @@ import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Stream; import java.util.stream.Stream.Builder; @@ -64,7 +69,7 @@ public class BufferTest { private static ExecutorService executor; private static final Memoize ALL_COMBINATIONS = new Memoize<>( - () -> fixtureCombinations().toArray(Fixture[]::new)); + () -> fixtureCombinations().filter(sample()).toArray(Fixture[]::new)); private static final Memoize NON_COMPOSITE = new Memoize<>( () -> Arrays.stream(ALL_COMBINATIONS.get()).filter(f -> !f.isComposite()).toArray(Fixture[]::new)); private static final Memoize HEAP_ALLOCS = new Memoize<>( @@ -73,6 +78,19 @@ public class BufferTest { () -> Arrays.stream(ALL_COMBINATIONS.get()).filter(f -> f.isDirect()).toArray(Fixture[]::new)); private static final Memoize POOLED_ALLOCS = new Memoize<>( () -> Arrays.stream(ALL_COMBINATIONS.get()).filter(f -> f.isPooled()).toArray(Fixture[]::new)); + private static final Memoize POOLED_DIRECT_ALLOCS = new Memoize<>( + () -> Arrays.stream(ALL_COMBINATIONS.get()).filter( + f -> f.isPooled() && f.isDirect()).toArray(Fixture[]::new)); + + private static Predicate sample() { + String sampleSetting = System.getProperty("sample"); + if ("nosample".equalsIgnoreCase(sampleSetting)) { + return fixture -> true; + } + Instant today = Instant.now().truncatedTo(ChronoUnit.DAYS); + SplittableRandom rng = new SplittableRandom(today.hashCode()); + return fixture -> rng.nextInt(0, 100) <= 2; // Filter out 97% of tests. + } static Fixture[] allocators() { return ALL_COMBINATIONS.get(); @@ -94,6 +112,10 @@ public class BufferTest { return POOLED_ALLOCS.get(); } + static Fixture[] pooledDirectAllocators() { + return POOLED_DIRECT_ALLOCS.get(); + } + static List initialAllocators() { return List.of( new Fixture("heap", BufferAllocator::heap, HEAP), @@ -962,7 +984,7 @@ public class BufferTest { } } - @Disabled + @Disabled // TODO @ParameterizedTest @MethodSource("allocators") public void sliceMustBecomeOwnedOnSourceBufferClose(Fixture fixture) { @@ -1650,42 +1672,42 @@ public class BufferTest { @Nested @Isolated class CleanerTests { - @Disabled("Precise native memory accounting does not work since recent panama-foreign changes.") @ParameterizedTest - @MethodSource("io.netty.buffer.api.BufTest#directAllocators") + @MethodSource("io.netty.buffer.api.BufferTest#directAllocators") public void bufferMustBeClosedByCleaner(Fixture fixture) throws InterruptedException { + var initial = NativeMemorySegmentManager.MEM_USAGE_NATIVE.sum(); var allocator = fixture.createAllocator(); allocator.close(); - int iterations = 100; + int iterations = 50; int allocationSize = 1024; for (int i = 0; i < iterations; i++) { allocateAndForget(allocator, allocationSize); System.gc(); System.runFinalization(); } - var sum = NativeMemorySegmentManager.MEM_USAGE_NATIVE.sum(); + var sum = NativeMemorySegmentManager.MEM_USAGE_NATIVE.sum() - initial; var totalAllocated = (long) allocationSize * iterations; assertThat(sum).isLessThan(totalAllocated); } - private void allocateAndForget(BufferAllocator allocator, int size) { + private static void allocateAndForget(BufferAllocator allocator, int size) { allocator.allocate(size); } - @Disabled("Precise native memory accounting does not work since recent panama-foreign changes.") @ParameterizedTest - @MethodSource("io.netty.buffer.api.BufTest#directPooledAllocators") + @MethodSource("io.netty.buffer.api.BufferTest#pooledDirectAllocators") public void buffersMustBeReusedByPoolingAllocatorEvenWhenDroppedByCleanerInsteadOfExplicitly(Fixture fixture) throws InterruptedException { + var initial = NativeMemorySegmentManager.MEM_USAGE_NATIVE.sum(); try (var allocator = fixture.createAllocator()) { - int iterations = 100; + int iterations = 50; int allocationSize = 1024; for (int i = 0; i < iterations; i++) { allocateAndForget(allocator, allocationSize); System.gc(); System.runFinalization(); } - var sum = NativeMemorySegmentManager.MEM_USAGE_NATIVE.sum(); + var sum = NativeMemorySegmentManager.MEM_USAGE_NATIVE.sum() - initial; var totalAllocated = (long) allocationSize * iterations; assertThat(sum).isLessThan(totalAllocated); }