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
Bit-Quill · Jan 4, 2024 · 050b4dd · 050b4dd
1 parent d21b8b3
commit 050b4dd
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 59 deletions.
diff --git a/java/maven/module-info-compiler-maven-plugin/pom.xml b/java/maven/module-info-compiler-maven-plugin/pom.xml
@@ -125,6 +125,4 @@
       </plugin>
     </plugins>
   </build>
-
-
-</project>
+</project>
diff --git a/java/memory/memory-core/pom.xml b/java/memory/memory-core/pom.xml
@@ -54,7 +54,7 @@
 
   <profiles>
     <profile>
-      <id>error-prone-jdk11+</id>
+      <id>jdk11+</id>
       <activation>
         <jdk>[11,]</jdk>
         <property>

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
@@ -442,94 +442,94 @@ 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);
           fail("allocated memory beyond max allowed");
         } 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/...mory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java b/...mory-unsafe/src/test/java/org/apache/arrow/memory/unsafe/TestUnsafeAllocationManager.java
@@ -22,17 +22,17 @@
 
 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;
 
 /**
  * Test cases for {@link UnsafeAllocationManager}.
  */
 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();