andygrove opened a new pull request, #6663: URL: https://github.com/apache/datafusion-comet/pull/6663
## Which issue does this PR close? None. This is the `branch-1.1` backport of #6552, which didn't close an issue either. ## Rationale for this change The JVM codegen dispatcher leaks off-heap Arrow memory for every output batch whose type is a top-level struct, such as the output of `from_json` or of a ScalaUDF that returns a case class or a tuple. The leak is 64 KiB per batch for `struct<string, int>` and 80 KiB for `struct<bigint, string>`. `RenamedStructVector`'s parent constructor builds a writer that allocates a child vector for each field, and `initializeChildrenFromFields` then replaces those children without closing them. The buffers come from `CometArrowAllocator`, which has no limit and is never closed, so nothing reports the leak, and the executor's off-heap memory just grows. `branch-1.1` has the same `allocateOutput` code and the same Arrow version (18.3.0), and the dispatcher is on by default here too (`spark.comet.exec.scalaUDF.codegen.enabled`). #6552 has the details. #6552 also has the `backport-1.0` label, and the commit applies to `branch-1.0` without conflicts. That backport isn't open yet. Under the backporting guide's newer-branch rule, it shouldn't merge before this one. ## What changes are included in this PR? A cherry-pick (`-x`) of #6552's commit. It applied without conflicts, and the patch is line-for-line the same as upstream's. - `RenamedStructVector` passes the `StructVector` constructor a field without children, so the writer creates no child vectors and the struct owns all the memory it allocates. `getField` returns the export field once `initializeChildrenFromFields` has run. - `allocateOutput` takes an `allocator` parameter that defaults to `CometArrowAllocator`. Existing callers are unchanged, and the new test uses the parameter to measure what a closed output leaves allocated. ## How are these changes tested? On this branch, with the default Spark 4.1 profile and JDK 17: - The codegen dispatcher suites all pass, 202 tests including the new one: `CometCodegenSuite` (106), `CometCodegenSourceSuite` (60), `CometCodegenFuzzSuite` (28), `CometCodegenHOFSuite` (5) and `CometSpecializedGettersDispatchSuite` (3). - `CometJsonJvmSuite`, `CometJsonExpressionSuite` and `CometScalaUDFClassLoaderSuite`, which cover `from_json` and case-class UDF outputs: 15 pass. - Without the fix, the new test fails on this branch. I put `RenamedStructVector` back to this branch's version and kept the new `allocator` parameter, which the test needs. The test then fails with `Memory was leaked by query. Memory leaked: (65536)` on `struct<name: string, age: int>`, so this branch has the leak and the test catches it. I didn't run the other Spark profiles locally. CI runs them on this branch. -- 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]
