ryux1 commented on PR #24319: URL: https://github.com/apache/datafusion/pull/24319#issuecomment-5302559183
Addressed the latest review feedback in b6414cd88 and rebased the branch onto current `main` (`47794ff24`). Changes since the last review: - moved buffer and recursive array accounting into `RecordBatchMemoryCounter` methods - removed runtime panic paths; current Arrow layouts are handled directly, while malformed/custom or future arrays use safe `ArrayData` traversal - added dedicated byte-view and list-view accounting plus concrete non-empty parity cases for `BinaryView`, `Utf8View`, `ListView`, and `LargeListView` - documented what each Criterion group measures - added the requested shared-buffer workload: one counter reused across 32 zero-copy slices, modeling the hash-join build side Fresh sequential A/B results against current `upstream/main` on the same machine: | Columns | `main` | PR | Speedup | |---:|---:|---:|---:| | 4 | 8.0954 µs | 1.1999 µs | 6.75x | | 16 | 31.417 µs | 5.7671 µs | 5.45x | | 64 | 124.67 µs | 23.229 µs | 5.37x | Validation: - `cargo fmt --all -- --check` - `cargo clippy --all-targets --all-features -- -D warnings` - `cargo test -p datafusion-common utils::memory --lib` (10 passed) - `cargo doc -p datafusion-common --no-deps` - focused Criterion benchmark above - extended workspace command: `core_integration` reached 1086 passed / 1 ignored before one process-RSS threshold test failed; the exact test fails identically on an isolated current-`main` worktree (branch 194.4 MB, `main` 194.8 MB, threshold 190.7 MB), so this is not introduced by the PR. A continuation with that one test skipped passed the 115 extended fuzz tests and subsequent package suites until the generic `--skip` option reached the custom SQLLogicTest binary, which does not accept it. All six review threads are addressed and resolved. @alamb @comphead, ready for another look when convenient. -- 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]
