From c455e67be4ee4671e6dae6d7f3454c0333844f2b Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 23 Nov 2023 12:31:40 -0800 Subject: [PATCH 01/19] Fix CData related test failures --- java/c/pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/java/c/pom.xml b/java/c/pom.xml index d66379d3566..8fc3f36994d 100644 --- a/java/c/pom.xml +++ b/java/c/pom.xml @@ -53,6 +53,11 @@ arrow-memory-unsafe test + + org.apache.arrow + arrow-format + test + com.google.guava guava From dd0647a54d65cca40a04ec6d744644fe06db27ba Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 24 Nov 2023 06:30:46 -0800 Subject: [PATCH 02/19] Refactor memory modules for JPMS support Based on #13072 - Avoid having multiple memory modules contribute to the same package - Introduce memory-netty-buffer-patch module for patching classes into Netty modules - Avoid using BaseAllocator in tests and use RootAllocator instead - Move TestBaseAllocator#testMemoryUsage() to a new test in memory-netty TestNettyAllocator because TestBaseAllocator is now in memory-core, but that specific test has Netty dependencies. --- .../apache/arrow/memory/CheckAllocator.java | 39 ++++- .../DefaultAllocationManagerOption.java | 12 +- .../memory/CountingAllocationListener.java | 4 + .../arrow/memory/TestBaseAllocator.java | 134 ++++++------------ .../arrow/memory/TestForeignAllocation.java | 7 + java/memory/memory-netty-buffer-patch/pom.xml | 44 ++++++ .../io/netty/buffer/ExpandableByteBuf.java | 0 .../java/io/netty/buffer/LargeBuffer.java | 0 .../netty/buffer/MutableWrappedByteBuf.java | 0 .../java/io/netty/buffer/NettyArrowBuf.java | 16 ++- .../netty/buffer/PooledByteBufAllocatorL.java | 7 +- .../buffer/UnsafeDirectLittleEndian.java | 0 .../memory/patch}/ArrowByteBufAllocator.java | 4 +- .../buffer/TestUnsafeDirectLittleEndian.java | 5 + java/memory/memory-netty/pom.xml | 7 + .../DefaultAllocationManagerFactory.java | 6 +- .../{ => netty}/NettyAllocationManager.java | 7 +- .../netty/buffer/TestExpandableByteBuf.java | 4 + .../io/netty/buffer/TestNettyArrowBuf.java | 5 +- .../{ => netty}/ITTestLargeArrowBuf.java | 5 +- .../TestAllocationManagerNetty.java | 4 +- .../memory/{ => netty}/TestEmptyArrowBuf.java | 9 +- .../memory/{ => netty}/TestEndianness.java | 4 +- .../TestNettyAllocationManager.java | 15 +- .../memory/netty/TestNettyAllocator.java | 75 ++++++++++ .../DefaultAllocationManagerFactory.java | 6 +- .../{ => unsafe}/UnsafeAllocationManager.java | 6 +- .../TestAllocationManagerUnsafe.java | 4 +- .../TestUnsafeAllocationManager.java | 13 +- java/memory/pom.xml | 1 + 30 files changed, 313 insertions(+), 130 deletions(-) rename java/memory/{memory-netty => memory-core}/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java (95%) rename java/memory/{memory-netty => memory-core}/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java (91%) rename java/memory/{memory-netty => memory-core}/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java (93%) create mode 100644 java/memory/memory-netty-buffer-patch/pom.xml rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/ExpandableByteBuf.java (100%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/LargeBuffer.java (100%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java (100%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/NettyArrowBuf.java (96%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java (97%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java (100%) rename java/memory/{memory-netty/src/main/java/org/apache/arrow/memory => memory-netty-buffer-patch/src/main/java/org/apache/arrow/memory/patch}/ArrowByteBufAllocator.java (97%) rename java/memory/{memory-netty => memory-netty-buffer-patch}/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java (95%) rename java/memory/memory-netty/src/main/java/org/apache/arrow/memory/{ => netty}/DefaultAllocationManagerFactory.java (87%) rename java/memory/memory-netty/src/main/java/org/apache/arrow/memory/{ => netty}/NettyAllocationManager.java (94%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/ITTestLargeArrowBuf.java (93%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/TestAllocationManagerNetty.java (90%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/TestEmptyArrowBuf.java (90%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/TestEndianness.java (92%) rename java/memory/memory-netty/src/test/java/org/apache/arrow/memory/{ => netty}/TestNettyAllocationManager.java (87%) create mode 100644 java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java rename java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/{ => unsafe}/DefaultAllocationManagerFactory.java (87%) rename java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/{ => unsafe}/UnsafeAllocationManager.java (89%) rename java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/{ => unsafe}/TestAllocationManagerUnsafe.java (90%) rename java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/{ => unsafe}/TestUnsafeAllocationManager.java (82%) diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/CheckAllocator.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/CheckAllocator.java index 79b825aa2e8..dac4a3fcff5 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/CheckAllocator.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/CheckAllocator.java @@ -31,20 +31,35 @@ */ final class CheckAllocator { private static final Logger logger = LoggerFactory.getLogger(CheckAllocator.class); - private static final String ALLOCATOR_PATH = "org/apache/arrow/memory/DefaultAllocationManagerFactory.class"; + // unique package names needed by JPMS module naming + private static final String ALLOCATOR_PATH_CORE = + "org/apache/arrow/memory/DefaultAllocationManagerFactory.class"; + private static final String ALLOCATOR_PATH_UNSAFE = + "org/apache/arrow/memory/unsafe/DefaultAllocationManagerFactory.class"; + private static final String ALLOCATOR_PATH_NETTY = + "org/apache/arrow/memory/netty/DefaultAllocationManagerFactory.class"; private CheckAllocator() { - } static String check() { Set urls = scanClasspath(); URL rootAllocator = assertOnlyOne(urls); reportResult(rootAllocator); - return "org.apache.arrow.memory.DefaultAllocationManagerFactory"; + if (rootAllocator.getPath().contains("memory-core") || + rootAllocator.getPath().contains("/org/apache/arrow/memory/core/")) { + return "org.apache.arrow.memory.DefaultAllocationManagerFactory"; + } else if (rootAllocator.getPath().contains("memory-unsafe") || + rootAllocator.getPath().contains("/org/apache/arrow/memory/unsafe/")) { + return "org.apache.arrow.memory.unsafe.DefaultAllocationManagerFactory"; + } else if (rootAllocator.getPath().contains("memory-netty") || + rootAllocator.getPath().contains("/org/apache/arrow/memory/netty/")) { + return "org.apache.arrow.memory.netty.DefaultAllocationManagerFactory"; + } else { + throw new IllegalStateException("Unknown allocation manager type to infer. Current: " + rootAllocator.getPath()); + } } - private static Set scanClasspath() { // LinkedHashSet appropriate here because it preserves insertion order // during iteration @@ -53,9 +68,21 @@ private static Set scanClasspath() { ClassLoader allocatorClassLoader = CheckAllocator.class.getClassLoader(); Enumeration paths; if (allocatorClassLoader == null) { - paths = ClassLoader.getSystemResources(ALLOCATOR_PATH); + paths = ClassLoader.getSystemResources(ALLOCATOR_PATH_CORE); + if (!paths.hasMoreElements()) { + paths = ClassLoader.getSystemResources(ALLOCATOR_PATH_UNSAFE); + } + if (!paths.hasMoreElements()) { + paths = ClassLoader.getSystemResources(ALLOCATOR_PATH_NETTY); + } } else { - paths = allocatorClassLoader.getResources(ALLOCATOR_PATH); + paths = allocatorClassLoader.getResources(ALLOCATOR_PATH_CORE); + if (!paths.hasMoreElements()) { + paths = allocatorClassLoader.getResources(ALLOCATOR_PATH_UNSAFE); + } + if (!paths.hasMoreElements()) { + paths = allocatorClassLoader.getResources(ALLOCATOR_PATH_NETTY); + } } while (paths.hasMoreElements()) { URL path = paths.nextElement(); diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java index d57b72ba415..6d408abcc34 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerOption.java @@ -19,6 +19,7 @@ import java.lang.reflect.Field; +import org.apache.arrow.util.VisibleForTesting; import org.checkerframework.checker.nullness.qual.Nullable; @@ -64,8 +65,13 @@ public enum AllocationManagerType { Unknown, } + /** + * Returns the default allocation manager type. + * @return the default allocation manager type. + */ @SuppressWarnings("nullness:argument") //enum types valueOf are implicitly non-null - static AllocationManagerType getDefaultAllocationManagerType() { + @VisibleForTesting + public static AllocationManagerType getDefaultAllocationManagerType() { AllocationManagerType ret = AllocationManagerType.Unknown; try { @@ -122,7 +128,7 @@ private static AllocationManager.Factory getFactory(String clazzName) { private static AllocationManager.Factory getUnsafeFactory() { try { - return getFactory("org.apache.arrow.memory.UnsafeAllocationManager"); + return getFactory("org.apache.arrow.memory.unsafe.UnsafeAllocationManager"); } catch (RuntimeException e) { throw new RuntimeException("Please add arrow-memory-unsafe to your classpath," + " No DefaultAllocationManager found to instantiate an UnsafeAllocationManager", e); @@ -131,7 +137,7 @@ private static AllocationManager.Factory getUnsafeFactory() { private static AllocationManager.Factory getNettyFactory() { try { - return getFactory("org.apache.arrow.memory.NettyAllocationManager"); + return getFactory("org.apache.arrow.memory.netty.NettyAllocationManager"); } catch (RuntimeException e) { throw new RuntimeException("Please add arrow-memory-netty to your classpath," + " No DefaultAllocationManager found to instantiate an NettyAllocationManager", e); diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java similarity index 95% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java rename to java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java index 78c78c8ad8c..f1dd7e92c5c 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java @@ -17,6 +17,10 @@ package org.apache.arrow.memory; +import org.apache.arrow.memory.AllocationListener; +import org.apache.arrow.memory.AllocationOutcome; +import org.apache.arrow.memory.BufferAllocator; + /** * Counting allocation listener. * It counts the number of times it has been invoked, and how much memory allocation it has seen diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java similarity index 91% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java rename to java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java index 7613d073f8c..365c84f5a2b 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java @@ -31,22 +31,16 @@ import java.util.Collection; import java.util.Collections; import java.util.Iterator; -import java.util.stream.Collectors; import org.apache.arrow.memory.AllocationOutcomeDetails.Entry; import org.apache.arrow.memory.rounding.RoundingPolicy; import org.apache.arrow.memory.rounding.SegmentRoundingPolicy; import org.apache.arrow.memory.util.AssertionUtil; +import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; import org.junit.jupiter.api.Assertions; -import org.slf4j.LoggerFactory; -import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; -import ch.qos.logback.classic.spi.ILoggingEvent; -import ch.qos.logback.core.read.ListAppender; -import io.netty.buffer.PooledByteBufAllocatorL; import sun.misc.Unsafe; public class TestBaseAllocator { @@ -448,73 +442,73 @@ public ArrowBuf empty() { @Test public void testRootAllocator_listeners() throws Exception { CountingAllocationListener l1 = new CountingAllocationListener(); - assertEquals(0, l1.getNumPreCalls()); - assertEquals(0, l1.getNumCalls()); - assertEquals(0, l1.getNumReleaseCalls()); - assertEquals(0, l1.getNumChildren()); - assertEquals(0, l1.getTotalMem()); + Assert.assertEquals(0, l1.getNumPreCalls()); + Assert.assertEquals(0, l1.getNumCalls()); + Assert.assertEquals(0, l1.getNumReleaseCalls()); + Assert.assertEquals(0, l1.getNumChildren()); + Assert.assertEquals(0, l1.getTotalMem()); CountingAllocationListener l2 = new CountingAllocationListener(); - assertEquals(0, l2.getNumPreCalls()); - assertEquals(0, l2.getNumCalls()); - assertEquals(0, l2.getNumReleaseCalls()); - assertEquals(0, l2.getNumChildren()); - assertEquals(0, l2.getTotalMem()); + Assert.assertEquals(0, l2.getNumPreCalls()); + Assert.assertEquals(0, l2.getNumCalls()); + Assert.assertEquals(0, l2.getNumReleaseCalls()); + Assert.assertEquals(0, l2.getNumChildren()); + Assert.assertEquals(0, l2.getTotalMem()); // root and first-level child share the first listener // second-level and third-level child share the second listener try (final RootAllocator rootAllocator = new RootAllocator(l1, MAX_ALLOCATION)) { try (final BufferAllocator c1 = rootAllocator.newChildAllocator("c1", 0, MAX_ALLOCATION)) { - assertEquals(1, l1.getNumChildren()); + Assert.assertEquals(1, l1.getNumChildren()); final ArrowBuf buf1 = c1.buffer(16); assertNotNull("allocation failed", buf1); - assertEquals(1, l1.getNumPreCalls()); - assertEquals(1, l1.getNumCalls()); - assertEquals(0, l1.getNumReleaseCalls()); - assertEquals(16, l1.getTotalMem()); + Assert.assertEquals(1, l1.getNumPreCalls()); + Assert.assertEquals(1, l1.getNumCalls()); + Assert.assertEquals(0, l1.getNumReleaseCalls()); + Assert.assertEquals(16, l1.getTotalMem()); buf1.getReferenceManager().release(); try (final BufferAllocator c2 = c1.newChildAllocator("c2", l2, 0, MAX_ALLOCATION)) { - assertEquals(2, l1.getNumChildren()); // c1 got a new child, so c1's listener (l1) is notified - assertEquals(0, l2.getNumChildren()); + Assert.assertEquals(2, l1.getNumChildren()); // c1 got a new child, so c1's listener (l1) is notified + Assert.assertEquals(0, l2.getNumChildren()); final ArrowBuf buf2 = c2.buffer(32); assertNotNull("allocation failed", buf2); - assertEquals(1, l1.getNumCalls()); - assertEquals(16, l1.getTotalMem()); - assertEquals(1, l2.getNumPreCalls()); - assertEquals(1, l2.getNumCalls()); - assertEquals(0, l2.getNumReleaseCalls()); - assertEquals(32, l2.getTotalMem()); + Assert.assertEquals(1, l1.getNumCalls()); + Assert.assertEquals(16, l1.getTotalMem()); + Assert.assertEquals(1, l2.getNumPreCalls()); + Assert.assertEquals(1, l2.getNumCalls()); + Assert.assertEquals(0, l2.getNumReleaseCalls()); + Assert.assertEquals(32, l2.getTotalMem()); buf2.getReferenceManager().release(); try (final BufferAllocator c3 = c2.newChildAllocator("c3", 0, MAX_ALLOCATION)) { - assertEquals(2, l1.getNumChildren()); - assertEquals(1, l2.getNumChildren()); + Assert.assertEquals(2, l1.getNumChildren()); + Assert.assertEquals(1, l2.getNumChildren()); final ArrowBuf buf3 = c3.buffer(64); assertNotNull("allocation failed", buf3); - assertEquals(1, l1.getNumPreCalls()); - assertEquals(1, l1.getNumCalls()); - assertEquals(1, l1.getNumReleaseCalls()); - assertEquals(16, l1.getTotalMem()); - assertEquals(2, l2.getNumPreCalls()); - assertEquals(2, l2.getNumCalls()); - assertEquals(1, l2.getNumReleaseCalls()); - assertEquals(32 + 64, l2.getTotalMem()); + Assert.assertEquals(1, l1.getNumPreCalls()); + Assert.assertEquals(1, l1.getNumCalls()); + Assert.assertEquals(1, l1.getNumReleaseCalls()); + Assert.assertEquals(16, l1.getTotalMem()); + Assert.assertEquals(2, l2.getNumPreCalls()); + Assert.assertEquals(2, l2.getNumCalls()); + Assert.assertEquals(1, l2.getNumReleaseCalls()); + Assert.assertEquals(32 + 64, l2.getTotalMem()); buf3.getReferenceManager().release(); } - assertEquals(2, l1.getNumChildren()); - assertEquals(0, l2.getNumChildren()); // third-level child removed + Assert.assertEquals(2, l1.getNumChildren()); + Assert.assertEquals(0, l2.getNumChildren()); // third-level child removed } - assertEquals(1, l1.getNumChildren()); // second-level child removed - assertEquals(0, l2.getNumChildren()); + Assert.assertEquals(1, l1.getNumChildren()); // second-level child removed + Assert.assertEquals(0, l2.getNumChildren()); } - assertEquals(0, l1.getNumChildren()); // first-level child removed + Assert.assertEquals(0, l1.getNumChildren()); // first-level child removed - assertEquals(2, l2.getNumReleaseCalls()); + Assert.assertEquals(2, l2.getNumReleaseCalls()); } } @Test public void testRootAllocator_listenerAllocationFail() throws Exception { CountingAllocationListener l1 = new CountingAllocationListener(); - assertEquals(0, l1.getNumCalls()); - assertEquals(0, l1.getTotalMem()); + Assert.assertEquals(0, l1.getNumCalls()); + Assert.assertEquals(0, l1.getTotalMem()); // Test attempts to allocate too much from a child whose limit is set to half of the max // allocation. The listener's callback triggers, expanding the child allocator's limit, so then // the allocation succeeds. @@ -527,14 +521,14 @@ public void testRootAllocator_listenerAllocationFail() throws Exception { } catch (OutOfMemoryException e) { // expected } - assertEquals(0, l1.getNumCalls()); - assertEquals(0, l1.getTotalMem()); + Assert.assertEquals(0, l1.getNumCalls()); + Assert.assertEquals(0, l1.getTotalMem()); l1.setExpandOnFail(c1, MAX_ALLOCATION); ArrowBuf arrowBuf = c1.buffer(MAX_ALLOCATION); assertNotNull("allocation failed", arrowBuf); - assertEquals(1, l1.getNumCalls()); - assertEquals(MAX_ALLOCATION, l1.getTotalMem()); + Assert.assertEquals(1, l1.getNumCalls()); + Assert.assertEquals(MAX_ALLOCATION, l1.getTotalMem()); arrowBuf.getReferenceManager().release(); } } @@ -1098,42 +1092,6 @@ public void testMemoryLeakWithReservation() throws Exception { } } - @Test - public void testMemoryUsage() { - ListAppender memoryLogsAppender = new ListAppender<>(); - Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator"); - try { - logger.setLevel(Level.TRACE); - logger.addAppender(memoryLogsAppender); - memoryLogsAppender.start(); - try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null, - 1024, new PooledByteBufAllocatorL().empty.memoryAddress())) { - buf.memoryAddress(); - } - boolean result = false; - long startTime = System.currentTimeMillis(); - while ((System.currentTimeMillis() - startTime) < 10000) { // 10 seconds maximum for time to read logs - result = memoryLogsAppender.list.stream() - .anyMatch( - log -> log.toString().contains("Memory Usage: \n") && - log.toString().contains("Large buffers outstanding: ") && - log.toString().contains("Normal buffers outstanding: ") && - log.getLevel().equals(Level.TRACE) - ); - if (result) { - break; - } - } - assertTrue("Log messages are:\n" + - memoryLogsAppender.list.stream().map(ILoggingEvent::toString).collect(Collectors.joining("\n")), - result); - } finally { - memoryLogsAppender.stop(); - logger.detachAppender(memoryLogsAppender); - logger.setLevel(null); - } - } - @Test public void testOverlimit() { try (BufferAllocator allocator = new RootAllocator(1024)) { diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java similarity index 93% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java rename to java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java index ec049ca6923..46e94fad37b 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestForeignAllocation.java @@ -23,6 +23,13 @@ import java.util.ArrayList; import java.util.List; +import org.apache.arrow.memory.AllocationListener; +import org.apache.arrow.memory.AllocationOutcome; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.ForeignAllocation; +import org.apache.arrow.memory.OutOfMemoryException; +import org.apache.arrow.memory.RootAllocator; import org.apache.arrow.memory.util.MemoryUtil; import org.junit.After; import org.junit.Before; diff --git a/java/memory/memory-netty-buffer-patch/pom.xml b/java/memory/memory-netty-buffer-patch/pom.xml new file mode 100644 index 00000000000..1d4407c638d --- /dev/null +++ b/java/memory/memory-netty-buffer-patch/pom.xml @@ -0,0 +1,44 @@ + + + + + arrow-memory + org.apache.arrow + 15.0.0-SNAPSHOT + + 4.0.0 + + arrow-memory-netty-buffer-patch + Arrow Memory - Netty Buffer + Netty Buffer needed to patch that is consumed by Arrow Memory Netty + + + + org.apache.arrow + arrow-memory-core + + + io.netty + netty-buffer + + + io.netty + netty-common + + + org.slf4j + slf4j-api + + + diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/ExpandableByteBuf.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/ExpandableByteBuf.java similarity index 100% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/ExpandableByteBuf.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/ExpandableByteBuf.java diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/LargeBuffer.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/LargeBuffer.java similarity index 100% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/LargeBuffer.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/LargeBuffer.java diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java similarity index 100% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/MutableWrappedByteBuf.java diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/NettyArrowBuf.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java similarity index 96% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/NettyArrowBuf.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java index 71e4b7cb6d5..466444c7d53 100644 --- a/java/memory/memory-netty/src/main/java/io/netty/buffer/NettyArrowBuf.java +++ b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/NettyArrowBuf.java @@ -17,8 +17,6 @@ package io.netty.buffer; -import static org.apache.arrow.memory.util.LargeMemoryUtil.checkedCastToInt; - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -29,10 +27,12 @@ import java.nio.channels.ScatteringByteChannel; import org.apache.arrow.memory.ArrowBuf; -import org.apache.arrow.memory.ArrowByteBufAllocator; import org.apache.arrow.memory.BoundsChecking; import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.patch.ArrowByteBufAllocator; +import org.apache.arrow.memory.util.LargeMemoryUtil; import org.apache.arrow.util.Preconditions; +import org.apache.arrow.util.VisibleForTesting; import io.netty.util.internal.PlatformDependent; @@ -264,7 +264,7 @@ public ByteBuffer nioBuffer(long index, int length) { * @return ByteBuffer */ private ByteBuffer getDirectBuffer(long index) { - return PlatformDependent.directBuffer(addr(index), checkedCastToInt(length - index)); + return PlatformDependent.directBuffer(addr(index), LargeMemoryUtil.checkedCastToInt(length - index)); } @Override @@ -580,11 +580,13 @@ public NettyArrowBuf setMedium(int index, int value) { } @Override + @VisibleForTesting protected void _setInt(int index, int value) { setInt(index, value); } @Override + @VisibleForTesting protected void _setIntLE(int index, int value) { this.chk(index, 4); PlatformDependent.putInt(this.addr(index), Integer.reverseBytes(value)); @@ -620,9 +622,9 @@ public static NettyArrowBuf unwrapBuffer(ArrowBuf buf) { final NettyArrowBuf nettyArrowBuf = new NettyArrowBuf( buf, buf.getReferenceManager().getAllocator(), - checkedCastToInt(buf.capacity())); - nettyArrowBuf.readerIndex(checkedCastToInt(buf.readerIndex())); - nettyArrowBuf.writerIndex(checkedCastToInt(buf.writerIndex())); + LargeMemoryUtil.checkedCastToInt(buf.capacity())); + nettyArrowBuf.readerIndex(LargeMemoryUtil.checkedCastToInt(buf.readerIndex())); + nettyArrowBuf.writerIndex(LargeMemoryUtil.checkedCastToInt(buf.writerIndex())); return nettyArrowBuf; } diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java similarity index 97% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java index ba9aba353c3..ea84a3258e8 100644 --- a/java/memory/memory-netty/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java +++ b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java @@ -17,13 +17,12 @@ package io.netty.buffer; -import static org.apache.arrow.memory.util.AssertionUtil.ASSERT_ENABLED; - import java.lang.reflect.Field; import java.nio.ByteBuffer; import java.util.concurrent.atomic.AtomicLong; import org.apache.arrow.memory.OutOfMemoryException; +import org.apache.arrow.memory.util.AssertionUtil; import org.apache.arrow.memory.util.LargeMemoryUtil; import io.netty.util.internal.OutOfDirectMemoryError; @@ -51,7 +50,7 @@ public PooledByteBufAllocatorL() { } /** - * Returns a {@linkplain io.netty.buffer.UnsafeDirectLittleEndian} of the given size. + * Returns a {@linkplain UnsafeDirectLittleEndian} of the given size. */ public UnsafeDirectLittleEndian allocate(long size) { try { @@ -180,7 +179,7 @@ private UnsafeDirectLittleEndian newDirectBufferL(int initialCapacity, int maxCa fail(); } - if (!ASSERT_ENABLED) { + if (!AssertionUtil.ASSERT_ENABLED) { return new UnsafeDirectLittleEndian((PooledUnsafeDirectByteBuf) buf); } diff --git a/java/memory/memory-netty/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java b/java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java similarity index 100% rename from java/memory/memory-netty/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java rename to java/memory/memory-netty-buffer-patch/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java diff --git a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/ArrowByteBufAllocator.java b/java/memory/memory-netty-buffer-patch/src/main/java/org/apache/arrow/memory/patch/ArrowByteBufAllocator.java similarity index 97% rename from java/memory/memory-netty/src/main/java/org/apache/arrow/memory/ArrowByteBufAllocator.java rename to java/memory/memory-netty-buffer-patch/src/main/java/org/apache/arrow/memory/patch/ArrowByteBufAllocator.java index ff40b49ff6f..6ce08b5a590 100644 --- a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/ArrowByteBufAllocator.java +++ b/java/memory/memory-netty-buffer-patch/src/main/java/org/apache/arrow/memory/patch/ArrowByteBufAllocator.java @@ -15,7 +15,9 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.patch; + +import org.apache.arrow.memory.BufferAllocator; import io.netty.buffer.AbstractByteBufAllocator; import io.netty.buffer.ByteBuf; diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java b/java/memory/memory-netty-buffer-patch/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java similarity index 95% rename from java/memory/memory-netty/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java rename to java/memory/memory-netty-buffer-patch/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java index c2bd95bb3d9..043c2c1605a 100644 --- a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java +++ b/java/memory/memory-netty-buffer-patch/src/test/java/io/netty/buffer/TestUnsafeDirectLittleEndian.java @@ -28,6 +28,11 @@ import org.junit.Test; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.LargeBuffer; +import io.netty.buffer.Unpooled; +import io.netty.buffer.UnsafeDirectLittleEndian; + public class TestUnsafeDirectLittleEndian { private static final boolean LITTLE_ENDIAN = ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN; diff --git a/java/memory/memory-netty/pom.xml b/java/memory/memory-netty/pom.xml index e625cbeabc6..159ab5160c9 100644 --- a/java/memory/memory-netty/pom.xml +++ b/java/memory/memory-netty/pom.xml @@ -26,9 +26,15 @@ org.apache.arrow arrow-memory-core + + org.apache.arrow + arrow-memory-netty-buffer-patch + ${project.version} + io.netty netty-buffer + provided io.netty @@ -37,6 +43,7 @@ org.slf4j slf4j-api + test ch.qos.logback diff --git a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java b/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/DefaultAllocationManagerFactory.java similarity index 87% rename from java/memory/memory-netty/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java rename to java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/DefaultAllocationManagerFactory.java index 10cfb5c1648..8ece77178f0 100644 --- a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java +++ b/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/DefaultAllocationManagerFactory.java @@ -15,7 +15,11 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; + +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; /** * The default Allocation Manager Factory for a module. diff --git a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/NettyAllocationManager.java b/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/NettyAllocationManager.java similarity index 94% rename from java/memory/memory-netty/src/main/java/org/apache/arrow/memory/NettyAllocationManager.java rename to java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/NettyAllocationManager.java index 20004778307..58354d0c2ee 100644 --- a/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/NettyAllocationManager.java +++ b/java/memory/memory-netty/src/main/java/org/apache/arrow/memory/netty/NettyAllocationManager.java @@ -15,7 +15,12 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; + +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.ReferenceManager; import io.netty.buffer.PooledByteBufAllocatorL; import io.netty.buffer.UnsafeDirectLittleEndian; diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java index b39cca8e8e7..67557b65a62 100644 --- a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java +++ b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestExpandableByteBuf.java @@ -23,6 +23,10 @@ import org.junit.Assert; import org.junit.Test; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.ExpandableByteBuf; +import io.netty.buffer.NettyArrowBuf; + public class TestExpandableByteBuf { @Test diff --git a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java index 45d3b41e8a6..f18bccb4c9d 100644 --- a/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java +++ b/java/memory/memory-netty/src/test/java/io/netty/buffer/TestNettyArrowBuf.java @@ -20,12 +20,15 @@ import java.nio.ByteBuffer; import org.apache.arrow.memory.ArrowBuf; -import org.apache.arrow.memory.ArrowByteBufAllocator; import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.RootAllocator; +import org.apache.arrow.memory.patch.ArrowByteBufAllocator; import org.junit.Assert; import org.junit.Test; +import io.netty.buffer.CompositeByteBuf; +import io.netty.buffer.NettyArrowBuf; + public class TestNettyArrowBuf { @Test diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/ITTestLargeArrowBuf.java similarity index 93% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/ITTestLargeArrowBuf.java index fa8d510e361..71dba73d289 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/ITTestLargeArrowBuf.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/ITTestLargeArrowBuf.java @@ -15,10 +15,13 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.RootAllocator; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestAllocationManagerNetty.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestAllocationManagerNetty.java similarity index 90% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestAllocationManagerNetty.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestAllocationManagerNetty.java index 2dbd56480b8..7f1e34ddc5f 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestAllocationManagerNetty.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestAllocationManagerNetty.java @@ -15,10 +15,12 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.DefaultAllocationManagerOption; import org.junit.Test; /** diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEmptyArrowBuf.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEmptyArrowBuf.java similarity index 90% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEmptyArrowBuf.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEmptyArrowBuf.java index 3fd7ce74aab..b9948083e6f 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEmptyArrowBuf.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEmptyArrowBuf.java @@ -15,10 +15,13 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.ReferenceManager; +import org.apache.arrow.memory.RootAllocator; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -47,8 +50,8 @@ public static void afterClass() { public void testZeroBuf() { // Exercise the historical log inside the empty ArrowBuf. This is initialized statically, and there is a circular // dependency between ArrowBuf and BaseAllocator, so if the initialization happens in the wrong order, the - // historical log will be null even though BaseAllocator.DEBUG is true. - allocator.getEmpty().print(new StringBuilder(), 0, BaseAllocator.Verbosity.LOG_WITH_STACKTRACE); + // historical log will be null even though RootAllocator.DEBUG is true. + allocator.getEmpty().print(new StringBuilder(), 0, RootAllocator.Verbosity.LOG_WITH_STACKTRACE); } @Test diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEndianness.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEndianness.java similarity index 92% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEndianness.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEndianness.java index dcaeb24889e..a782523cbc6 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestEndianness.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestEndianness.java @@ -15,12 +15,14 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; import java.nio.ByteOrder; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.RootAllocator; import org.junit.Test; import io.netty.buffer.ByteBuf; diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestNettyAllocationManager.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocationManager.java similarity index 87% rename from java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestNettyAllocationManager.java rename to java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocationManager.java index 1b64cd73363..39692c96ceb 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/TestNettyAllocationManager.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocationManager.java @@ -15,13 +15,18 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.netty; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.BufferLedger; +import org.apache.arrow.memory.RootAllocator; import org.junit.Test; /** @@ -31,8 +36,8 @@ public class TestNettyAllocationManager { static int CUSTOMIZED_ALLOCATION_CUTOFF_VALUE = 1024; - private BaseAllocator createCustomizedAllocator() { - return new RootAllocator(BaseAllocator.configBuilder() + private RootAllocator createCustomizedAllocator() { + return new RootAllocator(RootAllocator.configBuilder() .allocationManagerFactory(new AllocationManager.Factory() { @Override public AllocationManager create(BufferAllocator accountingAllocator, long size) { @@ -65,7 +70,7 @@ private void readWriteArrowBuf(ArrowBuf buffer) { @Test public void testSmallBufferAllocation() { final long bufSize = CUSTOMIZED_ALLOCATION_CUTOFF_VALUE - 512L; - try (BaseAllocator allocator = createCustomizedAllocator(); + try (RootAllocator allocator = createCustomizedAllocator(); ArrowBuf buffer = allocator.buffer(bufSize)) { assertTrue(buffer.getReferenceManager() instanceof BufferLedger); @@ -89,7 +94,7 @@ public void testSmallBufferAllocation() { @Test public void testLargeBufferAllocation() { final long bufSize = CUSTOMIZED_ALLOCATION_CUTOFF_VALUE + 1024L; - try (BaseAllocator allocator = createCustomizedAllocator(); + try (RootAllocator allocator = createCustomizedAllocator(); ArrowBuf buffer = allocator.buffer(bufSize)) { assertTrue(buffer.getReferenceManager() instanceof BufferLedger); BufferLedger bufferLedger = (BufferLedger) buffer.getReferenceManager(); diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java new file mode 100644 index 00000000000..b9525d0fe9d --- /dev/null +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.arrow.memory.netty; + +import static org.junit.Assert.assertTrue; + +import java.util.stream.Collectors; + +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.ReferenceManager; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import io.netty.buffer.PooledByteBufAllocatorL; + +/** + * Test netty allocators. + */ +public class TestNettyAllocator { + + @Test + public void testMemoryUsage() { + ListAppender memoryLogsAppender = new ListAppender<>(); + Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator"); + try { + logger.setLevel(Level.TRACE); + logger.addAppender(memoryLogsAppender); + memoryLogsAppender.start(); + try (ArrowBuf buf = new ArrowBuf(ReferenceManager.NO_OP, null, + 1024, new PooledByteBufAllocatorL().empty.memoryAddress())) { + buf.memoryAddress(); + } + boolean result = false; + long startTime = System.currentTimeMillis(); + while ((System.currentTimeMillis() - startTime) < 10000) { // 10 seconds maximum for time to read logs + result = memoryLogsAppender.list.stream() + .anyMatch( + log -> log.toString().contains("Memory Usage: \n") && + log.toString().contains("Large buffers outstanding: ") && + log.toString().contains("Normal buffers outstanding: ") && + log.getLevel().equals(Level.TRACE) + ); + if (result) { + break; + } + } + assertTrue("Log messages are:\n" + + memoryLogsAppender.list.stream().map(ILoggingEvent::toString).collect(Collectors.joining("\n")), + result); + } finally { + memoryLogsAppender.stop(); + logger.detachAppender(memoryLogsAppender); + logger.setLevel(null); + } + } +} diff --git a/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java b/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/DefaultAllocationManagerFactory.java similarity index 87% rename from java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java rename to java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/DefaultAllocationManagerFactory.java index 720c3d02d23..dfb6c706856 100644 --- a/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/DefaultAllocationManagerFactory.java +++ b/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/DefaultAllocationManagerFactory.java @@ -15,7 +15,11 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.unsafe; + +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; /** * The default Allocation Manager Factory for a module. diff --git a/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/UnsafeAllocationManager.java b/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/UnsafeAllocationManager.java similarity index 89% rename from java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/UnsafeAllocationManager.java rename to java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/UnsafeAllocationManager.java index b10aba3598d..3468a6ec65c 100644 --- a/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/UnsafeAllocationManager.java +++ b/java/memory/memory-unsafe/src/main/java/org/apache/arrow/memory/unsafe/UnsafeAllocationManager.java @@ -15,8 +15,12 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.unsafe; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; +import org.apache.arrow.memory.ReferenceManager; import org.apache.arrow.memory.util.MemoryUtil; /** diff --git a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestAllocationManagerUnsafe.java b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestAllocationManagerUnsafe.java similarity index 90% rename from java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestAllocationManagerUnsafe.java rename to java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestAllocationManagerUnsafe.java index 33abe92e50f..f1ca96eea0f 100644 --- a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestAllocationManagerUnsafe.java +++ b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestAllocationManagerUnsafe.java @@ -15,10 +15,12 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.unsafe; import static org.junit.Assert.assertEquals; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.DefaultAllocationManagerOption; import org.junit.Test; /** diff --git a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestUnsafeAllocationManager.java b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java similarity index 82% rename from java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestUnsafeAllocationManager.java rename to java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java index c15882a37a6..5d74c398e26 100644 --- a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/TestUnsafeAllocationManager.java +++ b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java @@ -15,11 +15,16 @@ * limitations under the License. */ -package org.apache.arrow.memory; +package org.apache.arrow.memory.unsafe; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import org.apache.arrow.memory.AllocationManager; +import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferLedger; +import org.apache.arrow.memory.RootAllocator; +import org.apache.arrow.memory.unsafe.UnsafeAllocationManager; import org.junit.Test; /** @@ -27,8 +32,8 @@ */ public class TestUnsafeAllocationManager { - private BaseAllocator createUnsafeAllocator() { - return new RootAllocator(BaseAllocator.configBuilder().allocationManagerFactory(UnsafeAllocationManager.FACTORY) + private RootAllocator createUnsafeAllocator() { + return new RootAllocator(RootAllocator.configBuilder().allocationManagerFactory(UnsafeAllocationManager.FACTORY) .build()); } @@ -51,7 +56,7 @@ private void readWriteArrowBuf(ArrowBuf buffer) { @Test public void testBufferAllocation() { final long bufSize = 4096L; - try (BaseAllocator allocator = createUnsafeAllocator(); + try (RootAllocator allocator = createUnsafeAllocator(); ArrowBuf buffer = allocator.buffer(bufSize)) { assertTrue(buffer.getReferenceManager() instanceof BufferLedger); BufferLedger bufferLedger = (BufferLedger) buffer.getReferenceManager(); diff --git a/java/memory/pom.xml b/java/memory/pom.xml index c10263b97f1..55fbb90353f 100644 --- a/java/memory/pom.xml +++ b/java/memory/pom.xml @@ -23,6 +23,7 @@ memory-core memory-unsafe + memory-netty-buffer-patch memory-netty From ceb41eb13f3800a7f0a7f7473a72350710d055ea Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 14:50:50 -0800 Subject: [PATCH 03/19] Add module-info for memory-core and memory-unsafe Also update arrow-vector to depend on memory-core. Netty in this PR omitted due to the complexity of needing to use patch-module on distributed JARs at run time. --- .../src/main/java/module-info.java | 28 +++++++++++++++++++ .../src/main/java/module-info.java | 20 +++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 java/memory/memory-core/src/main/java/module-info.java create mode 100644 java/memory/memory-unsafe/src/main/java/module-info.java diff --git a/java/memory/memory-core/src/main/java/module-info.java b/java/memory/memory-core/src/main/java/module-info.java new file mode 100644 index 00000000000..815f53bed63 --- /dev/null +++ b/java/memory/memory-core/src/main/java/module-info.java @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module org.apache.arrow.memory.core { + exports org.apache.arrow.memory; + exports org.apache.arrow.memory.rounding; + exports org.apache.arrow.memory.util; + exports org.apache.arrow.memory.util.hash; + exports org.apache.arrow.util; + requires transitive jdk.unsupported; + requires jsr305; + requires org.immutables.value; + requires slf4j.api; +} diff --git a/java/memory/memory-unsafe/src/main/java/module-info.java b/java/memory/memory-unsafe/src/main/java/module-info.java new file mode 100644 index 00000000000..b8701311b36 --- /dev/null +++ b/java/memory/memory-unsafe/src/main/java/module-info.java @@ -0,0 +1,20 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module org.apache.arrow.memory.unsafe { + requires org.apache.arrow.memory.core; +} From dcff64b0776bb5c1f5e4c1957c11447f9fbe2b3e Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 24 Nov 2023 10:05:18 -0800 Subject: [PATCH 04/19] Ignore TestBaseAllocator#testRootAllocator_getEmpty This test fails because we no longer use Netty's allocation manager factory since the test has moved into memory-core. To be fixed. --- .../src/test/java/org/apache/arrow/memory/TestBaseAllocator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java index 365c84f5a2b..f90e2214215 100644 --- a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java @@ -112,6 +112,7 @@ public void testRootAllocator_closeWithOutstanding() throws Exception { } @Test + @Ignore public void testRootAllocator_getEmpty() throws Exception { try (final RootAllocator rootAllocator = new RootAllocator(MAX_ALLOCATION)) { From e489d2684e4901d34d40ee073bc24989639d7322 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 28 Nov 2023 14:52:46 -0800 Subject: [PATCH 05/19] Edit TestArrowBuf#testEnabledHistoricalLog to have more info Log what message was reported if the message didn't contain the correct information. --- .../test/java/org/apache/arrow/memory/TestArrowBuf.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java index c88768c43bc..9ba42abc1ce 100644 --- a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestArrowBuf.java @@ -167,13 +167,15 @@ public void testEnabledHistoricalLog() { fieldDebug.set(null, true); try (BufferAllocator allocator = new RootAllocator(128)) { allocator.buffer(2); - Exception e = assertThrows(IllegalStateException.class, () -> allocator.close()); - assertTrue(e.getMessage().contains("event log for:")); // JDK8, JDK11 + Exception e = assertThrows(IllegalStateException.class, allocator::close); + assertTrue("Exception had the following message: " + e.getMessage(), + e.getMessage().contains("event log for:")); // JDK8, JDK11 } finally { fieldDebug.set(null, false); } } catch (Exception e) { - assertTrue(e.toString().contains("java.lang.NoSuchFieldException: modifiers")); // JDK17+ + assertTrue("Exception had the following toString(): " + e.toString(), + e.toString().contains("java.lang.NoSuchFieldException: modifiers")); // JDK17+ } finally { ((Logger) LoggerFactory.getLogger("org.apache.arrow")).setLevel(null); } From 3f9c45f010d2dd22cf9f98eb8e892742f5cb619a Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 15:02:58 -0800 Subject: [PATCH 06/19] Allow use of reflection on Unsafe from memory-core --- java/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/pom.xml b/java/pom.xml index 62e63d41a91..71ebf89416c 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -934,7 +934,7 @@ org.apache.maven.plugins maven-surefire-plugin - --add-opens=java.base/java.nio=ALL-UNNAMED + --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED From ee86308edc7f71fc77c8e7dab1902e31d6777b84 Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 15:23:09 -0800 Subject: [PATCH 07/19] Provide better error message about needing memory-core to have access to unsafe --- .../src/main/java/org/apache/arrow/memory/util/MemoryUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java index f79cf795312..2f74a985a3f 100644 --- a/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java +++ b/java/memory/memory-core/src/main/java/org/apache/arrow/memory/util/MemoryUtil.java @@ -148,7 +148,7 @@ public Object run() { // the static fields above get initialized final RuntimeException failure = new RuntimeException( "Failed to initialize MemoryUtil. You must start Java with " + - "`--add-opens=java.base/java.nio=ALL-UNNAMED` " + + "`--add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED` " + "(See https://arrow.apache.org/docs/java/install.html)", e); failure.printStackTrace(); throw failure; From f61bb9edaf514b0c3bf69518c5da4d06ffa2b7d7 Mon Sep 17 00:00:00 2001 From: James Duong Date: Wed, 29 Nov 2023 15:30:08 -0800 Subject: [PATCH 08/19] Allow memory-core to use reflection on memory-unsafe --- java/memory/memory-unsafe/src/main/java/module-info.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/java/memory/memory-unsafe/src/main/java/module-info.java b/java/memory/memory-unsafe/src/main/java/module-info.java index b8701311b36..aa340d21716 100644 --- a/java/memory/memory-unsafe/src/main/java/module-info.java +++ b/java/memory/memory-unsafe/src/main/java/module-info.java @@ -16,5 +16,7 @@ */ module org.apache.arrow.memory.unsafe { + exports org.apache.arrow.memory.unsafe to org.apache.arrow.memory.core; + requires org.apache.arrow.memory.core; } From aee387c03141edd77e21028dc2652d2a21258716 Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 30 Nov 2023 14:39:41 -0800 Subject: [PATCH 09/19] Add explicit dependency on immutables to memory-unsafe --- java/memory/memory-unsafe/pom.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/java/memory/memory-unsafe/pom.xml b/java/memory/memory-unsafe/pom.xml index 9f813730819..5ef4e8a9149 100644 --- a/java/memory/memory-unsafe/pom.xml +++ b/java/memory/memory-unsafe/pom.xml @@ -27,6 +27,10 @@ org.apache.arrow arrow-memory-core + + org.immutables + value + From a7dee75face8bc09d345aab802c979682616248b Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 30 Nov 2023 15:06:57 -0800 Subject: [PATCH 10/19] Enable reflection during arrow-memory-core tests The tests themselves (specifically testEnableHistoricalLog) use reflection on java.lang.reflect so surefire needs to enable this permission. --- java/memory/memory-core/pom.xml | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/java/memory/memory-core/pom.xml b/java/memory/memory-core/pom.xml index b914b1fa10d..f12f8a79624 100644 --- a/java/memory/memory-core/pom.xml +++ b/java/memory/memory-core/pom.xml @@ -46,7 +46,6 @@ org.apache.maven.plugins maven-surefire-plugin - @@ -58,6 +57,30 @@ + + error-prone-jdk11+ + + [11,] + + !m2e.version + + + + + + org.apache.maven.plugins + maven-surefire-plugin + + --add-opens=java.base/java.lang.reflect=org.apache.arrow.memory.core --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED + + + **/TestOpens.java + + + + + + opens-tests From 2847ee5c3f584896f8cea4faca599452300f7b06 Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 30 Nov 2023 15:56:00 -0800 Subject: [PATCH 11/19] Fix occasionally ConcurrentModificationException in Netty tests Due to using stream() on a log appender while the appender is still running and possibly adding more logs. --- .../memory/netty/TestNettyAllocator.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java index b9525d0fe9d..07fdc3f784e 100644 --- a/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java +++ b/java/memory/memory-netty/src/test/java/org/apache/arrow/memory/netty/TestNettyAllocator.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertTrue; +import java.util.Collections; import java.util.stream.Collectors; import org.apache.arrow.memory.ArrowBuf; @@ -40,6 +41,7 @@ public class TestNettyAllocator { @Test public void testMemoryUsage() { ListAppender memoryLogsAppender = new ListAppender<>(); + memoryLogsAppender.list = Collections.synchronizedList(memoryLogsAppender.list); Logger logger = (Logger) LoggerFactory.getLogger("arrow.allocator"); try { logger.setLevel(Level.TRACE); @@ -52,13 +54,17 @@ public void testMemoryUsage() { boolean result = false; long startTime = System.currentTimeMillis(); while ((System.currentTimeMillis() - startTime) < 10000) { // 10 seconds maximum for time to read logs - result = memoryLogsAppender.list.stream() - .anyMatch( - log -> log.toString().contains("Memory Usage: \n") && - log.toString().contains("Large buffers outstanding: ") && - log.toString().contains("Normal buffers outstanding: ") && - log.getLevel().equals(Level.TRACE) - ); + // Lock on the list backing the appender since a background thread might try to add more logs + // while stream() is iterating over list elements. This would throw a flakey ConcurrentModificationException. + synchronized (memoryLogsAppender.list) { + result = memoryLogsAppender.list.stream() + .anyMatch( + log -> log.toString().contains("Memory Usage: \n") && + log.toString().contains("Large buffers outstanding: ") && + log.toString().contains("Normal buffers outstanding: ") && + log.getLevel().equals(Level.TRACE) + ); + } if (result) { break; } From 37b48c1b349dec7313a61c61d36168cc80a2c2c0 Mon Sep 17 00:00:00 2001 From: James Duong Date: Mon, 11 Dec 2023 12:39:50 -0800 Subject: [PATCH 12/19] Correct the SLF4J JPMS module in memory-cpre --- java/memory/memory-core/src/main/java/module-info.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/memory/memory-core/src/main/java/module-info.java b/java/memory/memory-core/src/main/java/module-info.java index 815f53bed63..34ba34e80bc 100644 --- a/java/memory/memory-core/src/main/java/module-info.java +++ b/java/memory/memory-core/src/main/java/module-info.java @@ -24,5 +24,5 @@ requires transitive jdk.unsupported; requires jsr305; requires org.immutables.value; - requires slf4j.api; + requires org.slf4j; } From b5303d68f6e0b8c76db8cb468555411398c7108e Mon Sep 17 00:00:00 2001 From: James Duong Date: Mon, 11 Dec 2023 12:58:58 -0800 Subject: [PATCH 13/19] Add a test-only dependency on logback to memory-core Memory-core tests utilize logback. Note that this can't be added to the source module-info.java without adding an unnecessary dependency. It also can't be added to the test module-info.java without breaking the main module since the tests use the same packages. --- java/memory/memory-core/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/memory/memory-core/pom.xml b/java/memory/memory-core/pom.xml index f12f8a79624..3e24164d8c3 100644 --- a/java/memory/memory-core/pom.xml +++ b/java/memory/memory-core/pom.xml @@ -71,7 +71,7 @@ org.apache.maven.plugins maven-surefire-plugin - --add-opens=java.base/java.lang.reflect=org.apache.arrow.memory.core --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED + --add-reads=org.apache.arrow.memory.core=ch.qos.logback.classic --add-opens=java.base/java.lang.reflect=org.apache.arrow.memory.core --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED **/TestOpens.java From b1210798934ffb4ee8df5f9efd71c042e180485d Mon Sep 17 00:00:00 2001 From: James Duong Date: Thu, 4 Jan 2024 15:43:52 -0800 Subject: [PATCH 14/19] Address PR feedback - Fix newlines in module-info-complier-maven-plugin POm - Rename jdk11 profile to remove mention of error-prone - Avoid using l1 as a variable name in TestBaseAllocator - Avoid statically returning RootAllocator in TestUnsafeAllocationManager --- .../module-info-compiler-maven-plugin/pom.xml | 4 +- java/memory/memory-core/pom.xml | 2 +- .../arrow/memory/TestBaseAllocator.java | 104 +++++++++--------- .../unsafe/TestUnsafeAllocationManager.java | 6 +- 4 files changed, 57 insertions(+), 59 deletions(-) diff --git a/java/maven/module-info-compiler-maven-plugin/pom.xml b/java/maven/module-info-compiler-maven-plugin/pom.xml index 837a78b43b9..46c0d563f4e 100644 --- a/java/maven/module-info-compiler-maven-plugin/pom.xml +++ b/java/maven/module-info-compiler-maven-plugin/pom.xml @@ -125,6 +125,4 @@ - - - \ No newline at end of file + diff --git a/java/memory/memory-core/pom.xml b/java/memory/memory-core/pom.xml index 3e24164d8c3..6e411c0cd54 100644 --- a/java/memory/memory-core/pom.xml +++ b/java/memory/memory-core/pom.xml @@ -58,7 +58,7 @@ - error-prone-jdk11+ + jdk11+ [11,] diff --git a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java index f90e2214215..535d5c15e89 100644 --- a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/TestBaseAllocator.java @@ -442,79 +442,79 @@ public ArrowBuf empty() { @Test public void testRootAllocator_listeners() throws Exception { - CountingAllocationListener l1 = new CountingAllocationListener(); - Assert.assertEquals(0, l1.getNumPreCalls()); - Assert.assertEquals(0, l1.getNumCalls()); - Assert.assertEquals(0, l1.getNumReleaseCalls()); - Assert.assertEquals(0, l1.getNumChildren()); - Assert.assertEquals(0, l1.getTotalMem()); - CountingAllocationListener l2 = new CountingAllocationListener(); - Assert.assertEquals(0, l2.getNumPreCalls()); - Assert.assertEquals(0, l2.getNumCalls()); - Assert.assertEquals(0, l2.getNumReleaseCalls()); - Assert.assertEquals(0, l2.getNumChildren()); - Assert.assertEquals(0, l2.getTotalMem()); + CountingAllocationListener listener1 = new CountingAllocationListener(); + Assert.assertEquals(0, listener1.getNumPreCalls()); + Assert.assertEquals(0, listener1.getNumCalls()); + Assert.assertEquals(0, listener1.getNumReleaseCalls()); + Assert.assertEquals(0, listener1.getNumChildren()); + Assert.assertEquals(0, listener1.getTotalMem()); + CountingAllocationListener listener2 = new CountingAllocationListener(); + Assert.assertEquals(0, listener2.getNumPreCalls()); + Assert.assertEquals(0, listener2.getNumCalls()); + Assert.assertEquals(0, listener2.getNumReleaseCalls()); + Assert.assertEquals(0, listener2.getNumChildren()); + Assert.assertEquals(0, listener2.getTotalMem()); // root and first-level child share the first listener // second-level and third-level child share the second listener - try (final RootAllocator rootAllocator = new RootAllocator(l1, MAX_ALLOCATION)) { + try (final RootAllocator rootAllocator = new RootAllocator(listener1, MAX_ALLOCATION)) { try (final BufferAllocator c1 = rootAllocator.newChildAllocator("c1", 0, MAX_ALLOCATION)) { - Assert.assertEquals(1, l1.getNumChildren()); + Assert.assertEquals(1, listener1.getNumChildren()); final ArrowBuf buf1 = c1.buffer(16); assertNotNull("allocation failed", buf1); - Assert.assertEquals(1, l1.getNumPreCalls()); - Assert.assertEquals(1, l1.getNumCalls()); - Assert.assertEquals(0, l1.getNumReleaseCalls()); - Assert.assertEquals(16, l1.getTotalMem()); + Assert.assertEquals(1, listener1.getNumPreCalls()); + Assert.assertEquals(1, listener1.getNumCalls()); + Assert.assertEquals(0, listener1.getNumReleaseCalls()); + Assert.assertEquals(16, listener1.getTotalMem()); buf1.getReferenceManager().release(); - try (final BufferAllocator c2 = c1.newChildAllocator("c2", l2, 0, MAX_ALLOCATION)) { - Assert.assertEquals(2, l1.getNumChildren()); // c1 got a new child, so c1's listener (l1) is notified - Assert.assertEquals(0, l2.getNumChildren()); + try (final BufferAllocator c2 = c1.newChildAllocator("c2", listener2, 0, MAX_ALLOCATION)) { + Assert.assertEquals(2, listener1.getNumChildren()); // c1 got a new child, so listener1 is notified. + Assert.assertEquals(0, listener2.getNumChildren()); final ArrowBuf buf2 = c2.buffer(32); assertNotNull("allocation failed", buf2); - Assert.assertEquals(1, l1.getNumCalls()); - Assert.assertEquals(16, l1.getTotalMem()); - Assert.assertEquals(1, l2.getNumPreCalls()); - Assert.assertEquals(1, l2.getNumCalls()); - Assert.assertEquals(0, l2.getNumReleaseCalls()); - Assert.assertEquals(32, l2.getTotalMem()); + Assert.assertEquals(1, listener1.getNumCalls()); + Assert.assertEquals(16, listener1.getTotalMem()); + Assert.assertEquals(1, listener2.getNumPreCalls()); + Assert.assertEquals(1, listener2.getNumCalls()); + Assert.assertEquals(0, listener2.getNumReleaseCalls()); + Assert.assertEquals(32, listener2.getTotalMem()); buf2.getReferenceManager().release(); try (final BufferAllocator c3 = c2.newChildAllocator("c3", 0, MAX_ALLOCATION)) { - Assert.assertEquals(2, l1.getNumChildren()); - Assert.assertEquals(1, l2.getNumChildren()); + Assert.assertEquals(2, listener1.getNumChildren()); + Assert.assertEquals(1, listener2.getNumChildren()); final ArrowBuf buf3 = c3.buffer(64); assertNotNull("allocation failed", buf3); - Assert.assertEquals(1, l1.getNumPreCalls()); - Assert.assertEquals(1, l1.getNumCalls()); - Assert.assertEquals(1, l1.getNumReleaseCalls()); - Assert.assertEquals(16, l1.getTotalMem()); - Assert.assertEquals(2, l2.getNumPreCalls()); - Assert.assertEquals(2, l2.getNumCalls()); - Assert.assertEquals(1, l2.getNumReleaseCalls()); - Assert.assertEquals(32 + 64, l2.getTotalMem()); + Assert.assertEquals(1, listener1.getNumPreCalls()); + Assert.assertEquals(1, listener1.getNumCalls()); + Assert.assertEquals(1, listener1.getNumReleaseCalls()); + Assert.assertEquals(16, listener1.getTotalMem()); + Assert.assertEquals(2, listener2.getNumPreCalls()); + Assert.assertEquals(2, listener2.getNumCalls()); + Assert.assertEquals(1, listener2.getNumReleaseCalls()); + Assert.assertEquals(32 + 64, listener2.getTotalMem()); buf3.getReferenceManager().release(); } - Assert.assertEquals(2, l1.getNumChildren()); - Assert.assertEquals(0, l2.getNumChildren()); // third-level child removed + Assert.assertEquals(2, listener1.getNumChildren()); + Assert.assertEquals(0, listener2.getNumChildren()); // third-level child removed } - Assert.assertEquals(1, l1.getNumChildren()); // second-level child removed - Assert.assertEquals(0, l2.getNumChildren()); + Assert.assertEquals(1, listener1.getNumChildren()); // second-level child removed + Assert.assertEquals(0, listener2.getNumChildren()); } - Assert.assertEquals(0, l1.getNumChildren()); // first-level child removed + Assert.assertEquals(0, listener1.getNumChildren()); // first-level child removed - Assert.assertEquals(2, l2.getNumReleaseCalls()); + Assert.assertEquals(2, listener2.getNumReleaseCalls()); } } @Test public void testRootAllocator_listenerAllocationFail() throws Exception { - CountingAllocationListener l1 = new CountingAllocationListener(); - Assert.assertEquals(0, l1.getNumCalls()); - Assert.assertEquals(0, l1.getTotalMem()); + CountingAllocationListener listener1 = new CountingAllocationListener(); + Assert.assertEquals(0, listener1.getNumCalls()); + Assert.assertEquals(0, listener1.getTotalMem()); // Test attempts to allocate too much from a child whose limit is set to half of the max // allocation. The listener's callback triggers, expanding the child allocator's limit, so then // the allocation succeeds. try (final RootAllocator rootAllocator = new RootAllocator(MAX_ALLOCATION)) { - try (final BufferAllocator c1 = rootAllocator.newChildAllocator("c1", l1, 0, + try (final BufferAllocator c1 = rootAllocator.newChildAllocator("c1", listener1, 0, MAX_ALLOCATION / 2)) { try { c1.buffer(MAX_ALLOCATION); @@ -522,14 +522,14 @@ public void testRootAllocator_listenerAllocationFail() throws Exception { } catch (OutOfMemoryException e) { // expected } - Assert.assertEquals(0, l1.getNumCalls()); - Assert.assertEquals(0, l1.getTotalMem()); + Assert.assertEquals(0, listener1.getNumCalls()); + Assert.assertEquals(0, listener1.getTotalMem()); - l1.setExpandOnFail(c1, MAX_ALLOCATION); + listener1.setExpandOnFail(c1, MAX_ALLOCATION); ArrowBuf arrowBuf = c1.buffer(MAX_ALLOCATION); assertNotNull("allocation failed", arrowBuf); - Assert.assertEquals(1, l1.getNumCalls()); - Assert.assertEquals(MAX_ALLOCATION, l1.getTotalMem()); + Assert.assertEquals(1, listener1.getNumCalls()); + Assert.assertEquals(MAX_ALLOCATION, listener1.getTotalMem()); arrowBuf.getReferenceManager().release(); } } diff --git a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java index 5d74c398e26..77233e73cb3 100644 --- a/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java +++ b/java/memory/memory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java @@ -22,9 +22,9 @@ import org.apache.arrow.memory.AllocationManager; import org.apache.arrow.memory.ArrowBuf; +import org.apache.arrow.memory.BufferAllocator; import org.apache.arrow.memory.BufferLedger; import org.apache.arrow.memory.RootAllocator; -import org.apache.arrow.memory.unsafe.UnsafeAllocationManager; import org.junit.Test; /** @@ -32,7 +32,7 @@ */ public class TestUnsafeAllocationManager { - private RootAllocator createUnsafeAllocator() { + private BufferAllocator createUnsafeAllocator() { return new RootAllocator(RootAllocator.configBuilder().allocationManagerFactory(UnsafeAllocationManager.FACTORY) .build()); } @@ -56,7 +56,7 @@ private void readWriteArrowBuf(ArrowBuf buffer) { @Test public void testBufferAllocation() { final long bufSize = 4096L; - try (RootAllocator allocator = createUnsafeAllocator(); + try (BufferAllocator allocator = createUnsafeAllocator(); ArrowBuf buffer = allocator.buffer(bufSize)) { assertTrue(buffer.getReferenceManager() instanceof BufferLedger); BufferLedger bufferLedger = (BufferLedger) buffer.getReferenceManager(); From eec32bb0d620c3d2444d31fa203e8da86e2db0d9 Mon Sep 17 00:00:00 2001 From: James Duong Date: Fri, 5 Jan 2024 13:49:17 -0800 Subject: [PATCH 15/19] Update docs and archery tester for arrow-memory modules Update the command-line in the docs and the archery java tester to allow exposing reflection on unsafe on the org.apache.arrow.memory.core module --- dev/archery/archery/integration/tester_java.py | 5 +++-- docs/source/java/install.rst | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/dev/archery/archery/integration/tester_java.py b/dev/archery/archery/integration/tester_java.py index 6cd1afa64fe..032ac13e74e 100644 --- a/dev/archery/archery/integration/tester_java.py +++ b/dev/archery/archery/integration/tester_java.py @@ -40,7 +40,7 @@ def load_version_from_pom(): _JAVA_OPTS = [ "-Dio.netty.tryReflectionSetAccessible=true", "-Darrow.struct.conflict.policy=CONFLICT_APPEND", - "--add-opens=java.base/java.nio=ALL-UNNAMED", + "--add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED", # GH-39113: avoid failures accessing files in `/tmp/hsperfdata_...` "-XX:-UsePerfData", ] @@ -247,7 +247,8 @@ def __init__(self, *args, **kwargs): if 'Unrecognized option: --add-opens' not in proc.stderr: # Java 9+ self._java_opts.append( - '--add-opens=java.base/java.nio=ALL-UNNAMED') + '--add-opens=java.base/java.nio=' + 'org.apache.arrow.memory.core,ALL-UNNAMED') def _run(self, arrow_path=None, json_path=None, command='VALIDATE'): cmd = ( diff --git a/docs/source/java/install.rst b/docs/source/java/install.rst index 32c121573a6..b7484536f23 100644 --- a/docs/source/java/install.rst +++ b/docs/source/java/install.rst @@ -33,14 +33,14 @@ Java modules are compatible with JDK 8 and above. Currently, JDK 8, 11, 17, and 21 are tested in CI. When using Java 9 or later, some JDK internals must be exposed by -adding ``--add-opens=java.base/java.nio=ALL-UNNAMED`` to the ``java`` command: +adding ``--add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED`` to the ``java`` command: .. code-block:: shell # Directly on the command line - $ java --add-opens=java.base/java.nio=ALL-UNNAMED -jar ... + $ java --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED -jar ... # Indirectly via environment variables - $ env _JAVA_OPTIONS="--add-opens=java.base/java.nio=ALL-UNNAMED" java -jar ... + $ env _JAVA_OPTIONS="--add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED" java -jar ... Otherwise, you may see errors like ``module java.base does not "opens java.nio" to unnamed module``. From 6597ba930a0fc1c570390efda11f99ad87516fdf Mon Sep 17 00:00:00 2001 From: James Duong Date: Mon, 8 Jan 2024 12:03:57 -0800 Subject: [PATCH 16/19] Update maven-javadoc-plugin to 3.6.3 To support module-info.java files --- java/gandiva/pom.xml | 2 +- java/performance/pom.xml | 2 +- java/pom.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/java/gandiva/pom.xml b/java/gandiva/pom.xml index 128fa1508fb..e837a09ff83 100644 --- a/java/gandiva/pom.xml +++ b/java/gandiva/pom.xml @@ -79,7 +79,7 @@ org.apache.maven.plugins maven-javadoc-plugin - 2.9.1 + 3.6.3 attach-javadocs diff --git a/java/performance/pom.xml b/java/performance/pom.xml index d215d856d7a..5e0b6c1b545 100644 --- a/java/performance/pom.xml +++ b/java/performance/pom.xml @@ -195,7 +195,7 @@ maven-javadoc-plugin - 2.9.1 + 3.6.3 maven-resources-plugin diff --git a/java/pom.xml b/java/pom.xml index 71ebf89416c..6ece3eae2e2 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -768,7 +768,7 @@ org.apache.maven.plugins maven-javadoc-plugin - 3.0.0-M1 + 3.6.3 From fdd2c76a60d85b05670d2d332e03a8e2bf9ffceb Mon Sep 17 00:00:00 2001 From: James Duong Date: Mon, 8 Jan 2024 13:06:38 -0800 Subject: [PATCH 17/19] Disable javadoc generation on module-info.java --- java/pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/java/pom.xml b/java/pom.xml index 6ece3eae2e2..0fe76e5eed2 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -783,6 +783,11 @@ + + + **/module-info.java + + org.apache.maven.plugins From 0de759c905a7cab1aa6255db1ea6cebc4384b83a Mon Sep 17 00:00:00 2001 From: James Duong Date: Mon, 8 Jan 2024 16:47:45 -0800 Subject: [PATCH 18/19] Attempt to fix javadoc issues --- java/pom.xml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/java/pom.xml b/java/pom.xml index 0fe76e5eed2..042488a5b94 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -579,6 +579,17 @@ + + org.apache.maven.plugins + maven-javadoc-plugin + 3.6.3 + + 8 + + **/module-info.java + + + org.apache.arrow.maven.plugins module-info-compiler-maven-plugin @@ -785,7 +796,7 @@ - **/module-info.java + **/module-info.java From 98f94c8d022d1d45186131d9a5969401c3991ab7 Mon Sep 17 00:00:00 2001 From: James Duong Date: Tue, 9 Jan 2024 17:00:57 -0800 Subject: [PATCH 19/19] Fix null checking in CountingAllocationListener --- .../org/apache/arrow/memory/CountingAllocationListener.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java index f1dd7e92c5c..38e1a582b8c 100644 --- a/java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java +++ b/java/memory/memory-core/src/test/java/org/apache/arrow/memory/CountingAllocationListener.java @@ -20,6 +20,7 @@ import org.apache.arrow.memory.AllocationListener; import org.apache.arrow.memory.AllocationOutcome; import org.apache.arrow.memory.BufferAllocator; +import org.checkerframework.checker.nullness.qual.Nullable; /** * Counting allocation listener. @@ -34,6 +35,7 @@ final class CountingAllocationListener implements AllocationListener { private long totalMem; private long currentMem; private boolean expandOnFail; + @Nullable BufferAllocator expandAlloc; long expandLimit; @@ -62,6 +64,10 @@ public void onAllocation(long size) { @Override public boolean onFailedAllocation(long size, AllocationOutcome outcome) { if (expandOnFail) { + if (expandAlloc == null) { + throw new IllegalStateException("expandAlloc must be non-null because this " + + "listener is set to expand on failure."); + } expandAlloc.setLimit(expandLimit); return true; }