sunchao commented on code in PR #7356:
URL: https://github.com/apache/datafusion-comet/pull/7356#discussion_r4485813334
##########
Makefile:
##########
@@ -22,6 +22,11 @@ $(shell ./mvnw help:evaluate -q -DforceStdout
-Dexpression=extraJavaTestArgs $(P
endef
# Build optional Comet native features (like hdfs-opendal e.g)
+comma := ,
+# The jemalloc Maven profile also builds the native library with the jemalloc
allocator
+ifneq (,$(findstring jemalloc,$(PROFILES)))
+override COMET_FEATURES := $(if
$(COMET_FEATURES),$(COMET_FEATURES)$(comma))jemalloc
Review Comment:
Could this reject a conflicting allocator feature, or explicitly make
jemalloc the selected allocator? `make -n release PROFILES="-Pspark-4.1
-Pjemalloc" COMET_FEATURES=mimalloc` now emits `cargo build --release
--features=mimalloc,jemalloc`. In this head, the allocator-selection cfgs in
`native/core/src/lib.rs` deliberately choose `System` when both flags are
enabled. But `is_feature_enabled("jemalloc")` returns true solely from the
Cargo flag, so the new test passes and the INFO message claims jemalloc even
though the native library uses the system allocator.
I compiled the exact selection modules and feature helper with both flags:
`actual allocator = system`, `jemalloc feature/assertion = true`. This also
silently changes an existing mimalloc build to the system allocator when the
profile is added. Use the existing `allocator_backend::NAME` as the source of
truth for the diagnostic/assertion, and reject or resolve the conflicting
features before building. This probe checks the actual cfg selection; it does
not claim an allocator performance 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]
