From 0a66bfe90ae3812b617ed22c1a951a90db3d0c2a Mon Sep 17 00:00:00 2001 From: Dan King Date: Wed, 11 Oct 2023 16:11:26 -0400 Subject: [PATCH 1/2] [query] no unnecessary object allocations in RegionMemory.allocate Consider this: ```scala class Foo { def bar(): (Long, Long) = (3, 4) def destructure(): Unit = { val (x, y) = bar() } def accessors(): Unit = { val zz = bar() val x = zz._1 val y = zz._2 } } ``` These should be exactly equivalent, right? There's no way Scala would compile the match into something horrible. Right? Right? ``` public void destructure(); Code: 0: aload_0 1: invokevirtual #27 // Method bar:()Lscala/Tuple2; 4: astore_3 5: aload_3 6: ifnull 35 9: aload_3 10: invokevirtual #33 // Method scala/Tuple2._1$mcJ$sp:()J 13: lstore 4 15: aload_3 16: invokevirtual #36 // Method scala/Tuple2._2$mcJ$sp:()J 19: lstore 6 21: new #13 // class scala/Tuple2$mcJJ$sp 24: dup 25: lload 4 27: lload 6 29: invokespecial #21 // Method scala/Tuple2$mcJJ$sp."":(JJ)V 32: goto 47 35: goto 38 38: new #38 // class scala/MatchError 41: dup 42: aload_3 43: invokespecial #41 // Method scala/MatchError."":(Ljava/lang/Object;)V 46: athrow 47: astore_2 48: aload_2 49: invokevirtual #33 // Method scala/Tuple2._1$mcJ$sp:()J 52: lstore 8 54: aload_2 55: invokevirtual #36 // Method scala/Tuple2._2$mcJ$sp:()J 58: lstore 10 60: return public void accessors(); Code: 0: aload_0 1: invokevirtual #27 // Method bar:()Lscala/Tuple2; 4: astore_1 5: aload_1 6: invokevirtual #33 // Method scala/Tuple2._1$mcJ$sp:()J 9: lstore_2 10: aload_1 11: invokevirtual #36 // Method scala/Tuple2._2$mcJ$sp:()J 14: lstore 4 16: return ``` Yeah, so, it extracts the first and second elements of the primitive-specialized tuple, constructs a `(java.lang.Long, java.lang.Long)` Tuple, then does the match on that. sigh. --- hail/src/main/scala/is/hail/annotations/RegionMemory.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hail/src/main/scala/is/hail/annotations/RegionMemory.scala b/hail/src/main/scala/is/hail/annotations/RegionMemory.scala index 3261e4b70f7..c5e534f69a5 100644 --- a/hail/src/main/scala/is/hail/annotations/RegionMemory.scala +++ b/hail/src/main/scala/is/hail/annotations/RegionMemory.scala @@ -59,7 +59,9 @@ final class RegionMemory(pool: RegionPool) extends AutoCloseable { def getCurrentBlock(): Long = currentBlock private def allocateBigChunk(size: Long): Long = { - val (chunkPointer, chunkSize) = pool.getChunk(size) + val ret = pool.getChunk(size) + val chunkPointer = ret._1 + val chunkSize = ret._2 bigChunks.add(chunkPointer) totalChunkMemory += chunkSize chunkPointer From d4829aa3c4fd5130b36bda15874828a8e0dc08bd Mon Sep 17 00:00:00 2001 From: Dan King Date: Mon, 16 Oct 2023 11:44:58 -0400 Subject: [PATCH 2/2] admonition --- hail/src/main/scala/is/hail/annotations/RegionMemory.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hail/src/main/scala/is/hail/annotations/RegionMemory.scala b/hail/src/main/scala/is/hail/annotations/RegionMemory.scala index c5e534f69a5..9eb44c69a33 100644 --- a/hail/src/main/scala/is/hail/annotations/RegionMemory.scala +++ b/hail/src/main/scala/is/hail/annotations/RegionMemory.scala @@ -60,7 +60,7 @@ final class RegionMemory(pool: RegionPool) extends AutoCloseable { private def allocateBigChunk(size: Long): Long = { val ret = pool.getChunk(size) - val chunkPointer = ret._1 + val chunkPointer = ret._1 // Match expressions allocate https://github.com/hail-is/hail/pull/13794 val chunkSize = ret._2 bigChunks.add(chunkPointer) totalChunkMemory += chunkSize