From 2a1d7775990f39283a1f2ef2d072ef9ae4eac16f Mon Sep 17 00:00:00 2001
From: Metin Cakircali <metin.cakircali@ecmwf.int>
Date: Wed, 18 Sep 2024 11:43:21 +0200
Subject: [PATCH] fix(Key): reduce shared_ptr registry contention

copy and swap
---
 src/fdb5/database/Key.cc | 27 +++++++++++++--------------
 src/fdb5/database/Key.h  | 11 +++++------
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/fdb5/database/Key.cc b/src/fdb5/database/Key.cc
index be2668833..da4300a50 100644
--- a/src/fdb5/database/Key.cc
+++ b/src/fdb5/database/Key.cc
@@ -9,15 +9,14 @@
  */
 
 #include <algorithm>
+#include <utility>
 
 #include "eckit/container/DenseSet.h"
 #include "eckit/utils/Tokenizer.h"
 
 #include "metkit/mars/MarsRequest.h"
 
-#include "fdb5/config/Config.h"
 #include "fdb5/database/Key.h"
-#include "fdb5/LibFdb5.h"
 #include "fdb5/rules/Rule.h"
 #include "fdb5/rules/Schema.h"
 #include "fdb5/types/Type.h"
@@ -26,20 +25,19 @@ namespace fdb5 {
 
 //----------------------------------------------------------------------------------------------------------------------
 
-Key::Key(std::shared_ptr<const TypesRegistry> reg, bool canonical): registry_(reg), canonical_(canonical) { }
+Key::Key(std::shared_ptr<const TypesRegistry> reg, bool canonical): registry_(std::move(reg)), canonical_(canonical) { }
 
-Key::Key(const std::string& s, const Rule* rule): registry_(rule ? rule->registry() : nullptr), canonical_(false) {
+Key::Key(const std::string& s, const Rule* rule): registry_(rule ? rule->registry() : nullptr) {
 
-    eckit::Tokenizer parse(":", true);
+    eckit::Tokenizer  parse(":", true);
     eckit::StringList values;
     parse(s, values);
 
-    ASSERT(rule);
-    rule->fill(*this, values);
+    if (rule) { rule->fill(*this, values); }
 }
 
 Key::Key(const eckit::StringDict& keys, std::shared_ptr<const TypesRegistry> reg):
-    keys_(keys), registry_(reg), canonical_(false) {
+    keys_(keys), registry_(std::move(reg)) {
 
     eckit::StringDict::const_iterator it = keys.begin();
     eckit::StringDict::const_iterator end = keys.end();
@@ -48,12 +46,13 @@ Key::Key(const eckit::StringDict& keys, std::shared_ptr<const TypesRegistry> reg
     }
 }
 
-Key::Key(eckit::Stream& s, std::shared_ptr<const TypesRegistry> reg): registry_(reg), canonical_(reg == nullptr) {
+Key::Key(eckit::Stream& s, std::shared_ptr<const TypesRegistry> reg):
+    registry_(std::move(reg)), canonical_(registry_ == nullptr) {
     decode(s);
 }
 
 Key::Key(std::initializer_list<std::pair<const std::string, std::string>> l, std::shared_ptr<const TypesRegistry> reg):
-    keys_(l), registry_(reg), canonical_(reg == nullptr) {
+    keys_(l), registry_(std::move(reg)), canonical_(registry_ == nullptr) {
 
     for (const auto& kv : keys_) {
         names_.emplace_back(kv.first);
@@ -82,7 +81,8 @@ Key Key::parseString(const std::string& s, std::shared_ptr<const TypesRegistry>
 
     eckit::Tokenizer parse1(",");
     eckit::Tokenizer parse2("=");
-    Key ret(registry);
+
+    Key ret(std::move(registry));
 
     eckit::StringList vals;
     parse1(s, vals);
@@ -92,8 +92,7 @@ Key Key::parseString(const std::string& s, std::shared_ptr<const TypesRegistry>
         parse2(bit, kv);
         ASSERT(kv.size() == 2);
 
-        const Type &t = registry->lookupType(kv[0]);
-        std::string v = t.tidy(kv[0], kv[1]);
+        std::string v = ret.registry().lookupType(kv[0]).tidy(kv[0], kv[1]);
 
         if (ret.find(kv[0]) == ret.end()) {
             ret.push(kv[0], v);
@@ -297,7 +296,7 @@ bool Key::partialMatch(const metkit::mars::MarsRequest& request) const {
 }
 
 void Key::registry(std::shared_ptr<const TypesRegistry> reg) {
-    registry_ = reg;
+    registry_ = std::move(reg);
 }
 
 const TypesRegistry& Key::registry() const {
diff --git a/src/fdb5/database/Key.h b/src/fdb5/database/Key.h
index d20bf8667..b9b92f77c 100644
--- a/src/fdb5/database/Key.h
+++ b/src/fdb5/database/Key.h
@@ -18,7 +18,7 @@
 
 #include <map>
 #include <string>
-#include <vector>
+#include <utility>  // std::pair
 #include <set>
 #include <memory>
 
@@ -45,12 +45,11 @@ class Rule;
 class Key {
 
 public: // methods
-    explicit Key(std::shared_ptr<const TypesRegistry> reg = nullptr, bool canonical = false);
-    explicit Key(eckit::Stream&, std::shared_ptr<const TypesRegistry> reg = nullptr);
+    explicit Key(std::shared_ptr<const TypesRegistry> reg = {}, bool canonical = false);
+    explicit Key(eckit::Stream&, std::shared_ptr<const TypesRegistry> reg = {});
     explicit Key(const std::string &keys, const Rule* rule);
-    explicit Key(const eckit::StringDict& keys, std::shared_ptr<const TypesRegistry> reg = nullptr);
-    Key(std::initializer_list<std::pair<const std::string, std::string>>,
-        std::shared_ptr<const TypesRegistry> reg = nullptr);
+    explicit Key(const eckit::StringDict& keys, std::shared_ptr<const TypesRegistry> reg = {});
+    Key(std::initializer_list<std::pair<const std::string, std::string>>, std::shared_ptr<const TypesRegistry> reg = {});
 
     static Key parseStringUntyped(const std::string& s);
     /// @todo - this functionality should not be supported any more.