[GitHub] [geode-native] pdxcodemonkey opened a new pull request #733: GEODE-8887: Refactor EventIdTSS class

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

pdxcodemonkey opened a new pull request #733:
URL: https://github.com/apache/geode-native/pull/733


   - Use Meyers singleton pattern for global singleton (threadId), and
     thread local singleton EventIdTSS
   - rename all member variables in EventId and EventIdTSS classes
   - clean up logic slightly around global threadId atomic var
   - rename EventId getter methods for clarity
   
   @mreddington @dihardman @davebarnes97 @karensmolermiller 


----------------------------------------------------------------
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.

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

[GitHub] [geode-native] pdxcodemonkey merged pull request #733: GEODE-8887: Refactor EventIdTSS class

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

pdxcodemonkey merged pull request #733:
URL: https://github.com/apache/geode-native/pull/733


   


----------------------------------------------------------------
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.

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

[GitHub] [geode-native] pivotal-jbarrett commented on a change in pull request #733: GEODE-8887: Refactor EventIdTSS class

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

pivotal-jbarrett commented on a change in pull request #733:
URL: https://github.com/apache/geode-native/pull/733#discussion_r566597720



##########
File path: cppcache/src/EventId.cpp
##########
@@ -16,83 +16,99 @@
  */
 #include "EventId.hpp"
 
+#include <inttypes.h>
+
 #include <atomic>
 #include <cstring>
 
 #include <geode/DataInput.hpp>
 
 #include "ClientProxyMembershipID.hpp"
+#include "util/Log.hpp"
 
 namespace apache {
 namespace geode {
 namespace client {
 
-class EventIdTSS {
- private:
-  static std::atomic<int64_t> s_eidThrId;
-
-  int64_t m_eidThrTSS;
-  int64_t m_eidSeqTSS;
+class ThreadIdCounter {
+ public:
+  static std::atomic<int64_t>& instance() {
+    static std::atomic<int64_t> threadId_(0);
+    return threadId_;
+  }
 
-  ~EventIdTSS() = default;
-  EventIdTSS(const EventIdTSS&) = delete;
-  EventIdTSS& operator=(const EventIdTSS&) = delete;
+  static int64_t next() { return ++instance(); }
+};
 
+class EventIdTSS {
  public:
-  // this should get called just once per thread due to first access to TSS
-  EventIdTSS() {
-    m_eidThrTSS = ++s_eidThrId;
-    m_eidSeqTSS = 0;
+  static EventIdTSS& instance() {
+    thread_local EventIdTSS idTss_;

Review comment:
       Can we do even better on these types and variable names. 

##########
File path: cppcache/src/EventId.hpp
##########
@@ -20,12 +20,16 @@
 #ifndef GEODE_EVENTID_H_
 #define GEODE_EVENTID_H_
 
+#include <inttypes.h>

Review comment:
       Use <cstdint>.
   
   https://en.cppreference.com/w/cpp/header/cstdint




----------------------------------------------------------------
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.

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