cyb70289 commented on code in PR #13245:
URL: https://github.com/apache/arrow/pull/13245#discussion_r886347121
##########
cpp/src/arrow/ipc/reader.cc:
##########
@@ -1121,6 +1121,29 @@ class ARROW_EXPORT SelectiveIpcFileRecordBatchGenerator {
int index_;
};
+struct AtomicReadStats {
+ std::atomic<int64_t> num_messages{0};
+ std::atomic<int64_t> num_record_batches{0};
+ std::atomic<int64_t> num_dictionary_batches{0};
+ std::atomic<int64_t> num_dictionary_deltas{0};
+ std::atomic<int64_t> num_replaced_dictionaries{0};
+
+ /// \brief Capture a copy of the current counters
+ ///
+ /// It's possible to get inconsistent values. For example, if
+ /// this method is called in the middle of a read you might have
+ /// a case where num_messages != num_record_batches + num_dictionary_batches
Review Comment:
> In my limited understanding, since the values all likely reside in the
same cache line, there's likely false sharing anyways right? So we may as well
use the mutex.
Tested the effect of incrementing two different variables in same cache line
concurrently.
No obvious penalty for normal variables, I think most of the time the two
varible are in each cpu's store buffer, most load/store never reaches the L1
cache.
Big penalty for atomic fetch add, memory access is prefixed with `lock`
operand and have to reach L1, results in heavy cache line contentions. It's
much better if forcing the two variables in different cache line.
Benchmark result available in the source code.
https://gist.github.com/cyb70289/c9500d7f962a6f1d9b594ace9a9a358a
AFAIK, false sharing may cause trouble if global data is mutated by cpus in
different numa nodes.
--
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]
