This is an automated email from the ASF dual-hosted git repository.

leerho pushed a commit to branch fix_memory_leak
in repository https://gitbox.apache.org/repos/asf/datasketches-java.git

commit 737acc8ad99ac227ebb6231a995296a8d43a49fc
Author: Lee Rhodes <[email protected]>
AuthorDate: Fri Aug 16 18:26:09 2024 -0700

    Fix a memory leak in test.
---
 .../theta/DirectQuickSelectSketchTest.java         | 30 +++++++++++++++-------
 .../apache/datasketches/theta/UnionImplTest.java   | 14 +++++-----
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git 
a/src/test/java/org/apache/datasketches/theta/DirectQuickSelectSketchTest.java 
b/src/test/java/org/apache/datasketches/theta/DirectQuickSelectSketchTest.java
index ccfbf704..9b129daa 100644
--- 
a/src/test/java/org/apache/datasketches/theta/DirectQuickSelectSketchTest.java
+++ 
b/src/test/java/org/apache/datasketches/theta/DirectQuickSelectSketchTest.java
@@ -77,15 +77,17 @@ public class DirectQuickSelectSketchTest {
   @Test(expectedExceptions = SketchesArgumentException.class)
   public void checkConstructorKtooSmall() {
     int k = 8;
-    WritableMemory mem = makeNativeMemory(k);
-    UpdateSketch.builder().setNominalEntries(k).build(mem);
+    try (WritableMemory mem = makeNativeMemory(k)) {
+      UpdateSketch.builder().setNominalEntries(k).build(mem);
+    }
   }
 
   @Test(expectedExceptions = SketchesArgumentException.class)
   public void checkConstructorMemTooSmall() {
     int k = 16;
-    WritableMemory mem = makeNativeMemory(k/2);
-    UpdateSketch.builder().setNominalEntries(k).build(mem);
+    try (WritableMemory mem = makeNativeMemory(k/2)) {
+      UpdateSketch.builder().setNominalEntries(k).build(mem);
+    }
   }
 
   @Test(expectedExceptions = SketchesArgumentException.class)
@@ -589,7 +591,7 @@ public class DirectQuickSelectSketchTest {
     int u = 2*k;
     int memCapacity = (k << 4) + (Family.QUICKSELECT.getMinPreLongs() << 3);
 
-    try(WritableMemory mem = WritableMemory.allocateDirect(memCapacity)) {
+    try(WritableMemory mem = WritableMemory.allocateDirect(memCapacity)) { 
//will not request more memory
       UpdateSketch usk = 
UpdateSketch.builder().setNominalEntries(k).build(mem);
       DirectQuickSelectSketch sk1 = (DirectQuickSelectSketch)usk; //for 
internal checks
       assertTrue(usk.isEmpty());
@@ -599,6 +601,10 @@ public class DirectQuickSelectSketchTest {
       println(""+est);
       assertEquals(usk.getEstimate(), u, u*.05);
       assertTrue(sk1.getRetainedEntries(false) > k);
+      Memory mem2 = usk.getMemory();
+      assertTrue(mem2.isAlive());
+      assertTrue(mem2.isDirect()); //still off heap.
+      assertTrue(mem2.isSameResource(mem));
     }
   }
 
@@ -774,12 +780,14 @@ public class DirectQuickSelectSketchTest {
     int k = 1 << 12;
     int u = 2 * k;
     int bytes = Sketches.getMaxUpdateSketchBytes(k);
-    WritableMemory wmem = WritableMemory.allocateDirect(bytes/2); //will 
request
+    WritableMemory wmem = WritableMemory.allocateDirect(bytes/2); //will 
request more memory
     UpdateSketch sketch = 
Sketches.updateSketchBuilder().setNominalEntries(k).build(wmem);
     assertTrue(sketch.isSameResource(wmem));
     for (int i = 0; i < u; i++) { sketch.update(i); }
-    assertTrue(sketch.getMemory().isAlive());
-    assertFalse(wmem.isAlive());
+    Memory mem = sketch.getMemory();
+    assertTrue(mem.isAlive());
+    assertFalse(mem.isDirect()); //now on heap.
+    assertFalse(wmem.isAlive()); //wmem closed by MemoryRequestServer
   }
 
   @Test
@@ -787,7 +795,7 @@ public class DirectQuickSelectSketchTest {
     int k = 1 << 12;
     int u = 2 * k;
     int bytes = Sketches.getMaxUpdateSketchBytes(k);
-    WritableMemory wmem = WritableMemory.allocateDirect(bytes/2); //will 
request
+    WritableMemory wmem = WritableMemory.allocateDirect(bytes/2); //will 
request more memory
     UpdateSketch sketch = 
Sketches.updateSketchBuilder().setNominalEntries(k).build(wmem);
     for (int i = 0; i < u; i++) { sketch.update(i); }
     double est1 = sketch.getEstimate();
@@ -796,6 +804,10 @@ public class DirectQuickSelectSketchTest {
     UpdateSketch roSketch = (UpdateSketch) Sketches.wrapSketch(mem);
     double est2 = roSketch.getEstimate();
     assertEquals(est2, est1);
+    Memory mem2 = sketch.getMemory();
+    assertTrue(mem2.isAlive());
+    assertFalse(mem2.isDirect()); //now on heap
+    assertFalse(wmem.isAlive());  //wmem closed by MemoryRequestServer
     try {
       roSketch.rebuild();
       fail();
diff --git a/src/test/java/org/apache/datasketches/theta/UnionImplTest.java 
b/src/test/java/org/apache/datasketches/theta/UnionImplTest.java
index 31c26e4e..6fd88683 100644
--- a/src/test/java/org/apache/datasketches/theta/UnionImplTest.java
+++ b/src/test/java/org/apache/datasketches/theta/UnionImplTest.java
@@ -191,8 +191,8 @@ public class UnionImplTest {
     final int k = 1 << 12;
     final int u = 2 * k;
     final int bytes = Sketches.getMaxUpdateSketchBytes(k);
-    WritableMemory wmem = WritableMemory.allocateDirect(bytes / 2); //too 
small, forces new allocation on heap
-    WritableMemory wmem2 = WritableMemory.allocateDirect(bytes / 2);
+    WritableMemory wmem = WritableMemory.allocateDirect(bytes / 2); //not 
really used, except as a reference.
+    WritableMemory wmem2 = WritableMemory.allocateDirect(bytes / 2); //too 
small, forces new allocation on heap
     final UpdateSketch sketch = 
Sketches.updateSketchBuilder().setNominalEntries(k).build(wmem);
     assertTrue(sketch.isSameResource(wmem)); //also testing the isSameResource 
function
 
@@ -200,12 +200,12 @@ public class UnionImplTest {
     assertTrue(union.isSameResource(wmem2));
 
     for (int i = 0; i < u; i++) { union.update(i); }
-    assertFalse(union.isSameResource(wmem));
+    assertFalse(union.isSameResource(wmem)); //different Memories altogether
 
     final Union union2 = SetOperation.builder().buildUnion(); //on-heap union
     assertFalse(union2.isSameResource(wmem2));  //obviously not
-    wmem.close();
-    //note wmem2 has already been closed by the DefaultMemoryRequestServer
+    wmem.close(); //empty, but we must close it anyway.
+    //note wmem2 has already been closed by the DefaultMemoryRequestServer.
   }
 
   @Test
@@ -226,13 +226,13 @@ public class UnionImplTest {
     final double est1 = sk.getEstimate();
 
     final int bytes = 
Sketches.getMaxCompactSketchBytes(sk.getRetainedEntries(true));
-    try (WritableMemory wmem = WritableMemory.allocateDirect(bytes)) {
+    try (WritableMemory wmem = WritableMemory.allocateDirect(bytes)) { 
//sufficient memory
       final CompactSketch csk = sk.compact(true, wmem); //ordered, direct
       final Union union = Sketches.setOperationBuilder().buildUnion();
       union.union(csk);
       final double est2 = union.getResult().getEstimate();
       assertEquals(est2, est1);
-    }
+    } //wmem is closed here
   }
 
   @Test(expectedExceptions = SketchesArgumentException.class)


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to