[GitHub] [pulsar] BewareMyPower commented on a change in pull request #13079: [Issue 12844][pulsar-client-cpp] Excessive locking cause significant performance degradation

[GitBox <gi...@apache.org>]

BewareMyPower commented on a change in pull request #13079:
URL: https://github.com/apache/pulsar/pull/13079#discussion_r760740364



##########
File path: pulsar-client-cpp/lib/stats/ProducerStatsImpl.cc
##########
@@ -41,52 +42,60 @@ std::string ProducerStatsImpl::latencyToString(const LatencyAccumulator& obj) {
     return os.str();
 }
 
+struct ProducerStatsImpl::AsyncCallbackLock
+{
+   std::mutex mutex;
+   bool enabled = true;

Review comment:
       The similar question as before. BTW, I think we can wrap it in another header file. It only requires including `<mutex>` header so it's not a big issue.

##########
File path: pulsar-client-cpp/lib/stats/ConsumerStatsImpl.h
##########
@@ -30,30 +30,37 @@ class ConsumerStatsImpl : public ConsumerStatsBase {
    private:
     std::string consumerStr_;
 
-    unsigned long numBytesRecieved_ = 0;
-    std::map<Result, unsigned long> receivedMsgMap_;
-    std::map<std::pair<Result, proto::CommandAck_AckType>, unsigned long> ackedMsgMap_;
+    ExecutorServicePtr executor_;
+    DeadlineTimerPtr timer_;
+    unsigned int statsIntervalInSeconds_;
+
+    struct AsyncCallbackLock;
+    std::shared_ptr<AsyncCallbackLock> callbackLock_;
 
+    std::mutex rcvMutex_;
+    unsigned long numBytesRecieved_ = 0;
     unsigned long totalNumBytesRecieved_ = 0;

Review comment:
       I think we can correct the original typo as well? `numBytesRecieved_` -> `numBytesReceived_`.

##########
File path: pulsar-client-cpp/lib/stats/ConsumerStatsImpl.cc
##########
@@ -21,57 +21,84 @@
 #include <lib/LogUtils.h>
 
 #include <functional>
+#include <sstream>
 
 namespace pulsar {
 DECLARE_LOG_OBJECT();
 
+struct ConsumerStatsImpl::AsyncCallbackLock
+{
+    std::mutex mutex;
+    bool enabled = true;

Review comment:
       Should it be an `atomic_bool`?




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org