andygrove commented on code in PR #6856:
URL: https://github.com/apache/datafusion-comet/pull/6856#discussion_r4543642235


##########
spark/src/test/scala/org/apache/spark/sql/benchmark/CometC2RIsolatedBench.scala:
##########
@@ -92,15 +148,58 @@ object CometC2RIsolatedBench {
     batches.foreach(_.close())
   }
 
+  /** Reads every field of the flat schema. */
+  private def consumeFlat(u: InternalRow): Long =
+    u.getLong(0) + u.getInt(1) + u.getDouble(2).toLong + 
u.getUTF8String(3).numBytes()
+
+  /**
+   * Reads each nested column through its accessor, descending into the 
struct, array and map
+   * elements, so that the cost of materialising a nested value is part of the 
measurement.
+   */
+  private def consumeNested(u: InternalRow): Long = {
+    var sink = u.getLong(0)
+    if (!u.isNullAt(1)) {
+      val s = u.getStruct(1, 3)
+      sink += s.getInt(0) + s.getUTF8String(1).numBytes()
+      if (!s.isNullAt(2)) sink += s.getStruct(2, 2).getLong(1)
+    }
+    if (!u.isNullAt(2)) {
+      val a = u.getArray(2)
+      var i = 0
+      while (i < a.numElements()) {
+        if (!a.isNullAt(i)) sink += a.getInt(i)
+        i += 1
+      }
+    }
+    if (!u.isNullAt(3)) {
+      val a = u.getArray(3)
+      var i = 0
+      while (i < a.numElements()) {
+        sink += a.getUTF8String(i).numBytes()
+        i += 1
+      }
+    }
+    if (!u.isNullAt(4)) {
+      val m = u.getMap(4)
+      sink += m.numElements() + m.valueArray().getInt(0)
+    }
+    val structs = u.getArray(5)
+    var i = 0
+    while (i < structs.numElements()) {
+      val e = structs.getStruct(i, 2)
+      sink += e.getInt(0) + e.getUTF8String(1).numBytes()
+      i += 1
+    }
+    sink
+  }
+
   def main(args: Array[String]): Unit = {
-    val schema = new StructType()
-      .add("a", LongType)
-      .add("b", IntegerType)
-      .add("c", DoubleType)
-      .add("d", StringType)
-
-    runForBatchSize(schema, 8192)
-    runForBatchSize(schema, 512)
-    runForBatchSize(schema, 32)
+    runForBatchSize("flat", flatSchema, 8192, flatRow, consumeFlat)
+    runForBatchSize("flat", flatSchema, 512, flatRow, consumeFlat)
+    runForBatchSize("flat", flatSchema, 32, flatRow, consumeFlat)
+
+    runForBatchSize("nested", nestedSchema, 8192, nestedRow, consumeNested)
+    runForBatchSize("nested", nestedSchema, 512, nestedRow, consumeNested)
+    runForBatchSize("nested", nestedSchema, 32, nestedRow, consumeNested)

Review Comment:
   This no longer applies, because the nested case is gone from 
`CometC2RIsolatedBench`. #7505 removed the native converter that this benchmark 
compared with Spark's JVM path. `main` now compares the JVM path with 
`DirectColumnarToRowConverter`, which takes no nested types, so a nested schema 
would have only one arm. I merged `main` and took its version of the file, so 
the 32-row nested fixture (about 12 GiB) is not built. The nested C2R cases 
remain in `CometColumnarToRowBenchmark`, which reads Parquet and has no 
retained fixtures, and the description no longer lists the isolated bench 
(129e20b839).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to