xudong963 commented on code in PR #21820:
URL: https://github.com/apache/datafusion/pull/21820#discussion_r3654359752
##########
datafusion/common/src/hash_utils.rs:
##########
@@ -186,13 +188,32 @@ where
}).map_err(|_| _internal_datafusion_err!("with_hashes cannot access
thread-local storage during or after thread destruction"))?
}
+/// Creates hashes for the given arrays using a thread-local buffer and a
custom
+/// hash builder, then calls the provided callback with the computed hashes.
+///
+/// Hash compatibility with [`with_hashes`] follows the rules documented on
+/// [`create_hashes_with_hasher`].
+pub fn with_hashes_with_hasher<I, T, F, R, S>(
+ arrays: I,
+ hash_builder: &S,
+ callback: F,
+) -> Result<R>
+where
+ I: IntoIterator<Item = T>,
+ T: AsDynArray,
+ F: FnOnce(&[u64]) -> Result<R>,
+ S: BuildHasher,
+{
+ build_hasher::with_hashes_with_hasher(arrays, hash_builder, callback)
Review Comment:
I tried extracting the thread-local buffer lifecycle into a shared helper so
that `with_hashes` and `with_hashes_with_hasher` could reuse it. However, the
[benchmark](https://github.com/apache/datafusion/pull/21820#issuecomment-5086806857)
showed consistent regressions: approximately 13–16% for struct_array and 13%
for utf8: multiple, no nulls. After reverting the extraction, the [follow-up
benchmark](https://github.com/apache/datafusion/pull/21820#issuecomment-5087156917)
returned those cases to baseline.
The likely cause is the additional generic closure/function boundary around
this hot path, which changed inlining and code generation despite the helper
being marked #[inline]. Since this code is performance-sensitive and the
regression is material, I plan to retain this small amount of duplication to
preserve the existing optimized call structure.
--
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]
