Merge pull request #8538 from rgacogne/ddist-reduce-sholder-copies

Use move semantics when updating the content of the StateHolder

diff --git a/pdns/dnsdist-dynblocks.hh b/pdns/dnsdist-dynblocks.hh
index b71d8182802524f9daa43522ac4d0f2afd39842f..de585908b4b0433803b23b3f26cb91eee67d7b6a 100644 (file)
--- a/pdns/dnsdist-dynblocks.hh
+++ b/pdns/dnsdist-dynblocks.hh
@@ -282,7 +282,7 @@ public:
     }
 
     if (updated && blocks) {
-      g_dynblockNMG.setState(*blocks);
+      g_dynblockNMG.setState(std::move(*blocks));
     }
 
     if (!statNodeRoot.empty()) {
@@ -312,7 +312,7 @@ public:
           addOrRefreshBlockSMT(smtBlocks, now, name, d_suffixMatchRule, updated);
         }
         if (updated) {
-          g_dynblockSMT.setState(smtBlocks);
+          g_dynblockSMT.setState(std::move(smtBlocks));
         }
       }
     }

diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc
index 3cc60124c799e62a7f7e2e744f501abb50f79ff8..8312af301c96931635c910c8c4f79c294257cd84 100644 (file)
--- a/pdns/dnsdist-lua-actions.cc
+++ b/pdns/dnsdist-lua-actions.cc
@@ -1173,7 +1173,7 @@ private:
 };
 
 template<typename T, typename ActionT>
-static void addAction(GlobalStateHolder<vector<T> > *someRulActions, luadnsrule_t var, std::shared_ptr<ActionT> action, boost::optional<luaruleparams_t> params) {
+static void addAction(GlobalStateHolder<vector<T> > *someRulActions, const luadnsrule_t& var, const std::shared_ptr<ActionT>& action, boost::optional<luaruleparams_t>& params) {
   setLuaSideEffect();
 
   boost::uuids::uuid uuid;
@@ -1181,8 +1181,8 @@ static void addAction(GlobalStateHolder<vector<T> > *someRulActions, luadnsrule_
   parseRuleParams(params, uuid, creationOrder);
 
   auto rule=makeRule(var);
-  someRulActions->modify([rule, action, uuid, creationOrder](vector<T>& rulactions){
-      rulactions.push_back({rule, action, uuid, creationOrder});
+  someRulActions->modify([&rule, &action, &uuid, creationOrder](vector<T>& rulactions){
+      rulactions.push_back({std::move(rule), std::move(action), std::move(uuid), creationOrder});
     });
 }
 
@@ -1194,7 +1194,7 @@ void setupLuaActions()
       parseRuleParams(params, uuid, creationOrder);
 
       auto rule=makeRule(dnsrule);
-      DNSDistRuleAction ra({rule, action, uuid, creationOrder});
+      DNSDistRuleAction ra({std::move(rule), action, uuid, creationOrder});
       return std::make_shared<DNSDistRuleAction>(ra);
     });
 

diff --git a/pdns/dnsdist-lua-rules.cc b/pdns/dnsdist-lua-rules.cc
index f86acf4b2b5c170a91ea07c4767e7eb086563bcb..923c0fabfdc98c58b1fa074a0570c6b661b2392f 100644 (file)
--- a/pdns/dnsdist-lua-rules.cc
+++ b/pdns/dnsdist-lua-rules.cc
@@ -143,7 +143,7 @@ static void rmRule(GlobalStateHolder<vector<T> > *someRulActions, boost::variant
     }
     rules.erase(rules.begin()+*pos);
   }
-  someRulActions->setState(rules);
+  someRulActions->setState(std::move(rules));
 }
 
 template<typename T>
@@ -155,7 +155,7 @@ static void topRule(GlobalStateHolder<vector<T> > *someRulActions) {
   auto subject = *rules.rbegin();
   rules.erase(std::prev(rules.end()));
   rules.insert(rules.begin(), subject);
-  someRulActions->setState(rules);
+  someRulActions->setState(std::move(rules));
 }
 
 template<typename T>
@@ -175,7 +175,7 @@ static void mvRule(GlobalStateHolder<vector<T> > *someRespRulActions, unsigned i
       --to;
     rules.insert(rules.begin()+to, subject);
   }
-  someRespRulActions->setState(rules);
+  someRespRulActions->setState(std::move(rules));
 }
 
 void setupLuaRules()
@@ -259,7 +259,7 @@ void setupLuaRules()
             const auto& newruleaction = pair.second;
             if (newruleaction->d_action) {
               auto rule=makeRule(newruleaction->d_rule);
-              gruleactions.push_back({rule, newruleaction->d_action, newruleaction->d_id, newruleaction->d_creationOrder});
+              gruleactions.push_back({std::move(rule), newruleaction->d_action, newruleaction->d_id, newruleaction->d_creationOrder});
             }
           }
         });

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index ce634c3d0e9e4daff02845cab2106a8b2d195a39..7cdc320494c58dd069d9759b70c2f61783720313 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -4053,14 +4053,14 @@ static int serviceMain(int argc, char*argv[])
     for (const auto &p : parts) {
       dontThrottleNames.add(DNSName(p));
     }
-    g_dontThrottleNames.setState(dontThrottleNames);
+    g_dontThrottleNames.setState(std::move(dontThrottleNames));
 
     NetmaskGroup dontThrottleNetmasks;
     stringtok(parts, ::arg()["dont-throttle-netmasks"]);
     for (const auto &p : parts) {
       dontThrottleNetmasks.addMask(Netmask(p));
     }
-    g_dontThrottleNetmasks.setState(dontThrottleNetmasks);
+    g_dontThrottleNetmasks.setState(std::move(dontThrottleNetmasks));
   }
 
   s_balancingFactor = ::arg().asDouble("distribution-load-factor");

diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc
index db9c02f7502edcc304c899be6cb32e4d6d934539..339ea604f542de999945d9eea0a74a268428aa7f 100644 (file)
--- a/pdns/rec-lua-conf.cc
+++ b/pdns/rec-lua-conf.cc
@@ -570,7 +570,7 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de
 
   try {
     Lua.executeCode(ifs);
-    g_luaconfs.setState(lci);
+    g_luaconfs.setState(std::move(lci));
   }
   catch(const LuaContext::ExecutionErrorException& e) {
     g_log<<Logger::Error<<"Unable to load Lua script from '"+fname+"': ";

diff --git a/pdns/rec_channel_rec.cc b/pdns/rec_channel_rec.cc
index 0cad1f1ab44e1a713579dda2b46e6d6c9ff1fe5d..5acc4185190ae82f909968e4a65f947cbc1e2951 100644 (file)
--- a/pdns/rec_channel_rec.cc
+++ b/pdns/rec_channel_rec.cc
@@ -1476,7 +1476,7 @@ static string addDontThrottleNames(T begin, T end) {
     dnt.add(d);
   }
 
-  g_dontThrottleNames.setState(dnt);
+  g_dontThrottleNames.setState(std::move(dnt));
 
   ret += " to the list of nameservers that may not be throttled";
   g_log<<Logger::Info<<ret<<", requested via control channel"<<endl;
@@ -1515,7 +1515,7 @@ static string addDontThrottleNetmasks(T begin, T end) {
     dnt.addMask(t);
   }
 
-  g_dontThrottleNetmasks.setState(dnt);
+  g_dontThrottleNetmasks.setState(std::move(dnt));
 
   ret += " to the list of nameserver netmasks that may not be throttled";
   g_log<<Logger::Info<<ret<<", requested via control channel"<<endl;
@@ -1529,7 +1529,7 @@ static string clearDontThrottleNames(T begin, T end) {
 
   if (begin + 1 == end && *begin == "*"){
     SuffixMatchNode smn;
-    g_dontThrottleNames.setState(smn);
+    g_dontThrottleNames.setState(std::move(smn));
     string ret = "Cleared list of nameserver names that may not be throttled";
     g_log<<Logger::Warning<<ret<<", requested via control channel"<<endl;
     return ret + "\n";
@@ -1561,7 +1561,7 @@ static string clearDontThrottleNames(T begin, T end) {
     dnt.remove(name);
   }
 
-  g_dontThrottleNames.setState(dnt);
+  g_dontThrottleNames.setState(std::move(dnt));
 
   ret += " from the list of nameservers that may not be throttled";
   g_log<<Logger::Info<<ret<<", requested via control channel"<<endl;
@@ -1576,7 +1576,7 @@ static string clearDontThrottleNetmasks(T begin, T end) {
   if (begin + 1 == end && *begin == "*"){
     auto nmg = g_dontThrottleNetmasks.getCopy();
     nmg.clear();
-    g_dontThrottleNetmasks.setState(nmg);
+    g_dontThrottleNetmasks.setState(std::move(nmg));
 
     string ret = "Cleared list of nameserver addresses that may not be throttled";
     g_log<<Logger::Warning<<ret<<", requested via control channel"<<endl;
@@ -1613,7 +1613,7 @@ static string clearDontThrottleNetmasks(T begin, T end) {
     dnt.deleteMask(mask);
   }
 
-  g_dontThrottleNetmasks.setState(dnt);
+  g_dontThrottleNetmasks.setState(std::move(dnt));
 
   ret += " from the list of nameservers that may not be throttled";
   g_log<<Logger::Info<<ret<<", requested via control channel"<<endl;

diff --git a/pdns/sholder.hh b/pdns/sholder.hh
index 75837c30f1461280db8544bc3e1662faf91116ca..ce5da43ce38cba2b7e79769bb9985ea376aaa45c 100644 (file)
--- a/pdns/sholder.hh
+++ b/pdns/sholder.hh
@@ -92,12 +92,22 @@ public:
     return LocalStateHolder<T>(this);
   }
 
-  void setState(T state) //!< Safely & slowly change the global state
+  void setState(const T& state) //!< Safely & slowly change the global state
   {
     std::shared_ptr<T> newState = std::make_shared<T>(state);
     {
       std::lock_guard<std::mutex> l(d_lock);
-      d_state = newState;
+      d_state = std::move(newState);
+      d_generation++;
+    }
+  }
+
+  void setState(T&& state) //!< Safely & slowly change the global state
+  {
+    std::shared_ptr<T> newState = std::make_shared<T>(std::move(state));
+    {
+      std::lock_guard<std::mutex> l(d_lock);
+      d_state = std::move(newState);
       d_generation++;
     }
   }
@@ -111,10 +121,10 @@ public:
   //! Safely & slowly modify the global state
   template<typename F>
   void modify(F act) {
-    std::lock_guard<std::mutex> l(d_lock); 
+    std::lock_guard<std::mutex> l(d_lock);
     auto state=*d_state; // and yes, these three steps are necessary, can't ever modify state in place, even when locked!
     act(state);
-    d_state = std::make_shared<T>(state);
+    d_state = std::make_shared<T>(std::move(state));
     ++d_generation;
   }