[ 
https://issues.apache.org/jira/browse/GEODE-8213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17129411#comment-17129411
 ] 

ASF GitHub Bot commented on GEODE-8213:
---------------------------------------

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



##########
File path: cppcache/src/SerializationRegistry.cpp
##########
@@ -409,44 +411,56 @@ void TheTypeMap::bindDataSerializable(TypeFactoryMethod 
func, int32_t id) {
         "TheTypeMap::bind: Serialization type not implemented.");
   }
 
-  std::lock_guard<util::concurrent::spinlock_mutex> guard(
-      m_dataSerializableMapLock);
-  const auto& result = m_dataSerializableMap.emplace(id, func);
+  auto tempDataSerializableMap =
+      std::make_shared<std::unordered_map<int32_t, TypeFactoryMethod>>(
+          *m_dataSerializableMap);
+  const auto& result = tempDataSerializableMap->emplace(id, func);
   if (!result.second) {
     LOGERROR("A class with ID %d is already registered.", id);
     throw IllegalStateException("A class with given ID is already 
registered.");
   }
+  std::atomic_store(&m_dataSerializableMap, tempDataSerializableMap);

Review comment:
       The `std::shared_ptr` makes this tricky and 
`std::atomic<std::shared_ptr>` doesn't come until C++20. :(
   
   Options:
   * Use raw pointers and manage the memory manually. (not great, memory leaks)
   * Use a lock-free map from 3rd part library. (adds new dependency)
   * Use `boost::shared_mutex` around existing maps. (fast reads, blocking 
write)
   
   I would go with the `boost::shared_mutex` approach, which can be replaced 
with `std::shared_mutex` in future. We only need to take the write exclusive 
lock to update the map which shouldn't be all the frequent. 




----------------------------------------------------------------
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:
us...@infra.apache.org


> C++ native client performance bottleneck in access to serialization registry
> ----------------------------------------------------------------------------
>
>                 Key: GEODE-8213
>                 URL: https://issues.apache.org/jira/browse/GEODE-8213
>             Project: Geode
>          Issue Type: Improvement
>          Components: native client
>            Reporter: Alberto Gomez
>            Assignee: Alberto Gomez
>            Priority: Major
>             Fix For: 1.14.0
>
>
> It's been observed that when the number of threads used in a Geode client 
> using PdxSerialization is greater than 8, there is an important drop in 
> performance.
> Analysing the client process behavior with perf, it has been observed a very 
> high CPU consumption by a spinlock 
> (apache::geode::util::concurrent::spinlock_mutex::lock) used when accessing 
> the serialization registry .



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to