From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 16 Jun 2020 11:56:57 +0000 (+0200)
Subject: Cleanup cache cleaner pruneCollection function
X-Git-Tag: dnsdist-1.5.0-rc4~20^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F9236%2Fhead;p=thirdparty%2Fpdns.git

Cleanup cache cleaner pruneCollection function
- use size_t instead of unsigned int for sizes
- const correctness
- layout
- simplify seond loop and use return value of erase(), it points to the next value
---

diff --git a/pdns/cachecleaner.hh b/pdns/cachecleaner.hh
index 7d42274bff..5b2ff3d6fc 100644
--- a/pdns/cachecleaner.hh
+++ b/pdns/cachecleaner.hh
@@ -27,66 +27,50 @@
 // this function can clean any cache that has a getTTD() method on its entries, a preRemoval() method and a 'sequence' index as its second index
 // the ritual is that the oldest entries are in *front* of the sequence collection, so on a hit, move an item to the end
 // on a miss, move it to the beginning
-template <typename S, typename C, typename T> void pruneCollection(C& container, T& collection, unsigned int maxCached, unsigned int scanFraction=1000)
+template <typename S, typename C, typename T> void pruneCollection(C& container, T& collection, size_t maxCached, size_t scanFraction = 1000)
 {
-  time_t now=time(0);
-  unsigned int toTrim=0;
-  
-  unsigned int cacheSize=collection.size();
+  const time_t now = time(0);
+  size_t toTrim = 0;
+  const size_t cacheSize = collection.size();
 
-  if(cacheSize > maxCached) {
+  if (cacheSize > maxCached) {
     toTrim = cacheSize - maxCached;
   }
 
-//  cout<<"Need to trim "<<toTrim<<" from cache to meet target!\n";
-
-  typedef typename T::template index<S>::type sequence_t;
-  sequence_t& sidx=collection.template get<S>();
-
-  unsigned int tried=0, lookAt, erased=0;
+  auto& sidx = collection.template get<S>();
 
   // two modes - if toTrim is 0, just look through 1/scanFraction of all records 
   // and nuke everything that is expired
   // otherwise, scan first 5*toTrim records, and stop once we've nuked enough
-  if(toTrim)
-    lookAt=5*toTrim;
-  else
-    lookAt=cacheSize/scanFraction;
+  const size_t lookAt = toTrim ? 5 * toTrim : cacheSize / scanFraction;
+  size_t tried = 0, erased = 0;
 
-  typename sequence_t::iterator iter=sidx.begin(), eiter;
-  for(; iter != sidx.end() && tried < lookAt ; ++tried) {
-    if(iter->getTTD() < now) {
+  for (auto iter = sidx.begin(); iter != sidx.end() && tried < lookAt ; ++tried) {
+    if (iter->getTTD() < now) {
       container.preRemoval(*iter);
       iter = sidx.erase(iter);
       erased++;
     }
-    else
+    else {
       ++iter;
+    }
 
-    if(toTrim && erased >= toTrim)
+    if (toTrim && erased >= toTrim) {
       break;
+    }
   }
 
-  //cout<<"erased "<<erased<<" records based on ttd\n";
-  
-  if(erased >= toTrim) // done
+  if (erased >= toTrim) { // done
     return;
+  }
 
   toTrim -= erased;
 
-  //if(toTrim)
-    // cout<<"Still have "<<toTrim - erased<<" entries left to erase to meet target\n"; 
-
-  eiter=iter=sidx.begin();
-  std::advance(eiter, toTrim);
   // just lob it off from the beginning
-  for (auto i = iter; ; ) {
-    if (i == eiter) {
-      break;
-    }
-
-    container.preRemoval(*i);
-    sidx.erase(i++);
+  auto iter = sidx.begin();
+  for (size_t i = 0; i < toTrim && iter != sidx.end(); i++) {
+    container.preRemoval(*iter);
+    iter = sidx.erase(iter);
   }
 }
 
diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc
index 1b952ba196..f4d145fffc 100644
--- a/pdns/recpacketcache.cc
+++ b/pdns/recpacketcache.cc
@@ -212,7 +212,7 @@ uint64_t RecursorPacketCache::bytes()
   return sum;
 }
 
-void RecursorPacketCache::doPruneTo(unsigned int maxCached)
+void RecursorPacketCache::doPruneTo(size_t maxCached)
 {
   pruneCollection<SequencedTag>(*this, d_packetCache, maxCached);
 }
diff --git a/pdns/recpacketcache.hh b/pdns/recpacketcache.hh
index 790d714fb8..75adce7d2a 100644
--- a/pdns/recpacketcache.hh
+++ b/pdns/recpacketcache.hh
@@ -56,7 +56,7 @@ public:
   bool getResponsePacket(unsigned int tag, const std::string& queryPacket, const DNSName& qname, uint16_t qtype, uint16_t qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, uint16_t* ecsBegin, uint16_t* ecsEnd, RecProtoBufMessage* protobufMessage);
   bool getResponsePacket(unsigned int tag, const std::string& queryPacket, DNSName& qname, uint16_t* qtype, uint16_t* qclass, time_t now, std::string* responsePacket, uint32_t* age, vState* valState, uint32_t* qhash, uint16_t* ecsBegin, uint16_t* ecsEnd, RecProtoBufMessage* protobufMessage);
   void insertResponsePacket(unsigned int tag, uint32_t qhash, std::string&& query, const DNSName& qname, uint16_t qtype, uint16_t qclass, std::string&& responsePacket, time_t now, uint32_t ttl, const vState& valState, uint16_t ecsBegin, uint16_t ecsEnd, boost::optional<RecProtoBufMessage>&& protobufMessage);
-  void doPruneTo(unsigned int maxSize=250000);
+  void doPruneTo(size_t maxSize=250000);
   uint64_t doDump(int fd);
   int doWipePacketCache(const DNSName& name, uint16_t qtype=0xffff, bool subtree=false);
   
diff --git a/pdns/recursordist/negcache.cc b/pdns/recursordist/negcache.cc
index a48107e761..1af402b7e3 100644
--- a/pdns/recursordist/negcache.cc
+++ b/pdns/recursordist/negcache.cc
@@ -183,7 +183,7 @@ void NegCache::clear()
  *
  * \param maxEntries The maximum number of entries that may exist in the cache.
  */
-void NegCache::prune(unsigned int maxEntries)
+void NegCache::prune(size_t maxEntries)
 {
   pruneCollection<SequenceTag>(*this, d_negcache, maxEntries, 200);
 }
diff --git a/pdns/recursordist/negcache.hh b/pdns/recursordist/negcache.hh
index 55200ff4a2..249725a375 100644
--- a/pdns/recursordist/negcache.hh
+++ b/pdns/recursordist/negcache.hh
@@ -68,7 +68,7 @@ public:
   bool getRootNXTrust(const DNSName& qname, const struct timeval& now, const NegCacheEntry** ne);
   uint64_t count(const DNSName& qname) const;
   uint64_t count(const DNSName& qname, const QType qtype) const;
-  void prune(unsigned int maxEntries);
+  void prune(size_t maxEntries);
   void clear();
   uint64_t dumpToFile(FILE* fd);
   uint64_t wipe(const DNSName& name, bool subtree = false);