zhuqi-lucas commented on code in PR #22751:
URL: https://github.com/apache/datafusion/pull/22751#discussion_r3355572761
##########
datafusion/physical-plan/Cargo.toml:
##########
@@ -76,6 +81,9 @@ datafusion-physical-expr = { workspace = true,
default-features = true }
datafusion-physical-expr-common = { workspace = true }
datafusion-proto-common = { workspace = true, optional = true }
datafusion-proto-models = { workspace = true, optional = true }
+# Optional heap profiler used by the `dhat-heap` feature for memory
Review Comment:
Good point @alamb, thanks for review, I agree. The dhat dep is shaky to take
on upstream when the tests it backs are not in CI. Will drop the dhat-heap
feature, the dhat dependency, the global_allocator wiring in lib.rs, and the
dhat_tests.
The factory extraction, the Time32 / Time64 supported_type tightening, and
the supported_type / make_group_column consistency fuzz test will all stay.
For the subsequent PRs in the #22706 split (FixedSizeList, Struct, List,
LargeList, composition tests), I will measure memory savings via
`GroupColumn::size()` head-to-head against `GroupValuesRows` (the
pattern that #22706 already uses for
`column_path_uses_less_memory_than_rows_for_*` tests). Those run in normal
cargo test so they will be exercised by CI.
The dhat-based profiling will live on my local for manual investigation
only. And later, we can add those to example or benchmark.
--
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]
