[
https://issues.apache.org/jira/browse/GEODE-8887?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17274162#comment-17274162
]
ASF GitHub Bot commented on GEODE-8887:
---------------------------------------
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:
[email protected]
> Switch EventId thread and sequence logic to Meyers singleton pattern
> --------------------------------------------------------------------
>
> Key: GEODE-8887
> URL: https://issues.apache.org/jira/browse/GEODE-8887
> Project: Geode
> Issue Type: Improvement
> Components: native client
> Reporter: Blake Bender
> Assignee: Blake Bender
> Priority: Major
> Labels: pull-request-available
>
> As a developer, I need to be able to rely on the order of initialization of
> my global and/or thread local singleton objects, in order to reasonably
> expect things to work that count on said objects. The current EventIdTSS
> implementation doesn't use Meyers singleton, and thus the order of
> initialization shows up seriously different from what you'd imagine. In
> particular, the thread local EventIdTSS object _appears_ to be initialized
> long before it is used on each thread, _and_ many of the threads the NC spins
> up will _never_ use it, leaving us with a couple of dozen or more extra
> copies of this thing lying around. Switching to the standard C++ singleton
> pattern will make this code a lot more deterministic, in addition to much
> more readable.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
