bkietz commented on a change in pull request #10632:
URL: https://github.com/apache/arrow/pull/10632#discussion_r661696226
##########
File path: cpp/src/arrow/compute/exec/expression.cc
##########
@@ -294,19 +293,21 @@ size_t Expression::hash() const {
}
auto call = CallNotNull(*this);
- if (call->hash != nullptr) {
- return call->hash->load();
+ const auto cached_hash = call->hash.load();
Review comment:
Instead, could we simply avoid lazy hashing?
https://github.com/apache/arrow/pull/10397/files?file-filters%5B%5D=.cc&file-filters%5B%5D=.cpp&file-filters%5B%5D=.h&file-filters%5B%5D=.pxd&file-filters%5B%5D=.pyx#diff-1c7baea2c704ebf6ec38232e30aa84900e7ea8f81d6faa0f138c30cd3e4ee7ecR45-R51
This will remove the need to write Call constructors too
##########
File path: cpp/src/arrow/util/async_generator.h
##########
@@ -1338,11 +1342,34 @@ class BackgroundGenerator {
return next;
}
+#if (defined(__GNUC__) || defined(__clang__))
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wclass-memaccess"
+#endif
+
+ void SetWorkerThreadId(const std::thread::id tid) {
+ uint64_t equiv{0};
+ // std::thread::id is trivially copyable as per C++ spec, so this should
work
Review comment:
```suggestion
// std::thread::id is trivially copyable as per C++ spec,
// so type punning as a uint64_t should work
static_assert(sizeof(std::thread::id) <= sizeof(uint64_t),
"std::thread::id can't fit into uint64_t");
```
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
