From ac0995bbb50f35671afcde475bbee2bfd4f024e3 Mon Sep 17 00:00:00 2001 From: bert hubert Date: Tue, 16 Aug 2016 10:36:08 +0200 Subject: [PATCH] remove our badly handcrafted AtomicCounter implementation, centralize packetcache cache cleaning interval (ready for further improvement) --- pdns/misc.hh | 52 +++---------------------------------------- pdns/packetcache.cc | 26 ++++++++-------------- pdns/packetcache.hh | 7 +++++- pdns/pdns_recursor.cc | 2 +- pdns/responsestats.cc | 3 +-- pdns/statbag.cc | 10 ++++----- pdns/statbag.hh | 6 ++--- 7 files changed, 28 insertions(+), 78 deletions(-) diff --git a/pdns/misc.hh b/pdns/misc.hh index 890fb6487a..d14f3a9c4e 100644 --- a/pdns/misc.hh +++ b/pdns/misc.hh @@ -36,6 +36,7 @@ using namespace ::boost::multi_index; #include "dns.hh" +#include #include #include #include @@ -368,57 +369,10 @@ inline bool pdns_iequals_ch(const char a, const char b) return true; } -// lifted from boost, with thanks -class AtomicCounter -{ -public: - typedef unsigned long native_t; - explicit AtomicCounter( native_t v = 0) : value_( v ) {} - - native_t operator++() - { - return atomic_exchange_and_add( &value_, +1 ) + 1; - } - - native_t operator++(int) - { - return atomic_exchange_and_add( &value_, +1 ); - } - - native_t operator+=(native_t val) - { - return atomic_exchange_and_add( &value_, val ); - } - - native_t operator-=(native_t val) - { - return atomic_exchange_and_add( &value_, -val ); - } - native_t operator--() - { - return atomic_exchange_and_add( &value_, -1 ) - 1; - } - - operator native_t() const - { - return atomic_exchange_and_add( &value_, 0); - } - - AtomicCounter(AtomicCounter const &rhs) : value_(rhs) - { - } - -private: - mutable native_t value_; - - static native_t atomic_exchange_and_add( native_t * pw, native_t dv ) - { - return __sync_fetch_and_add(pw, dv); - } -}; +typedef std::atomic AtomicCounter ; -// FIXME400 this should probably go? +// FIXME400 this should probably go? struct CIStringCompare: public std::binary_function { bool operator()(const string& a, const string& b) const diff --git a/pdns/packetcache.cc b/pdns/packetcache.cc index 2d0ec82fa1..87a6632da2 100644 --- a/pdns/packetcache.cc +++ b/pdns/packetcache.cc @@ -1,6 +1,6 @@ /* PowerDNS Versatile Database Driven Nameserver - Copyright (C) 2002 - 2014 PowerDNS.COM BV + Copyright (C) 2002 - 2016 PowerDNS.COM BV This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License version 2 as @@ -73,9 +73,7 @@ int PacketCache::get(DNSPacket *p, DNSPacket *cached, bool recursive) if(d_ttl<0) getTTLS(); - if(!((++d_ops) % 300000)) { - cleanup(); - } + cleanupIfNeeded(); if(d_doRecursion && p->d.rd) { // wants recursion if(!d_recursivettl) { @@ -163,9 +161,7 @@ void PacketCache::insert(DNSPacket *q, DNSPacket *r, bool recursive, unsigned in void PacketCache::insert(const DNSName &qname, const QType& qtype, CacheEntryType cet, const string& value, unsigned int ttl, int zoneID, bool meritsRecursion, unsigned int maxReplyLen, bool dnssecOk, bool EDNS) { - if(!((++d_ops) % 300000)) { - cleanup(); - } + cleanupIfNeeded(); if(!ttl) return; @@ -201,9 +197,7 @@ void PacketCache::insert(const DNSName &qname, const QType& qtype, CacheEntryTyp void PacketCache::insert(const DNSName &qname, const QType& qtype, CacheEntryType cet, const vector& value, unsigned int ttl, int zoneID) { - if(!((++d_ops) % 300000)) { - cleanup(); - } + cleanupIfNeeded(); if(!ttl) return; @@ -247,7 +241,7 @@ int PacketCache::purge() delcount+=mc.d_map.size(); mc.d_map.clear(); } - *d_statnumentries=AtomicCounter(0); + d_statnumentries->store(0); return delcount; } @@ -300,9 +294,7 @@ bool PacketCache::getEntry(const DNSName &qname, const QType& qtype, CacheEntryT if(d_ttl<0) getTTLS(); - if(!((++d_ops) % 300000)) { - cleanup(); - } + cleanupIfNeeded(); auto& mc=getMap(qname); @@ -392,7 +384,7 @@ int PacketCache::size() /** readlock for figuring out which iterators to delete, upgrade to writelock when actually cleaning */ void PacketCache::cleanup() { - *d_statnumentries=AtomicCounter(0); + d_statnumentries->store(0); for(auto& mc : d_maps) { ReadLock l(&mc.d_mut); @@ -401,7 +393,7 @@ void PacketCache::cleanup() unsigned int maxCached=::arg().asNum("max-cache-entries"); unsigned int toTrim=0; - AtomicCounter::native_t cacheSize=*d_statnumentries; + unsigned long cacheSize=*d_statnumentries; if(maxCached && cacheSize > maxCached) { toTrim = cacheSize - maxCached; @@ -444,7 +436,7 @@ void PacketCache::cleanup() // if(totErased) // cerr<<"erased: "<store(0); for(auto& mc : d_maps) { ReadLock l(&mc.d_mut); *d_statnumentries+=mc.d_map.size(); diff --git a/pdns/packetcache.hh b/pdns/packetcache.hh index 93d2193299..74e3770b24 100644 --- a/pdns/packetcache.hh +++ b/pdns/packetcache.hh @@ -1,6 +1,6 @@ /* PowerDNS Versatile Database Driven Nameserver - Copyright (C) 2002 - 2011 PowerDNS.COM BV + Copyright (C) 2002 - 2016 PowerDNS.COM BV This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License version 2 @@ -68,6 +68,11 @@ public: int size(); //!< number of entries in the cache + void cleanupIfNeeded() + { + if(!(++d_ops % 300000)) + cleanup(); + } void cleanup(); //!< force the cache to preen itself from expired packets int purge(); int purge(const std::string& match); // could be $ terminated. Is not a dnsname! diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index 6717e0571d..529cd7dce7 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -2783,7 +2783,7 @@ try time_t last_carbon=0; time_t carbonInterval=::arg().asNum("carbon-interval"); - counter=AtomicCounter(0); // used to periodically execute certain tasks + counter.store(0); // used to periodically execute certain tasks for(;;) { while(MT->schedule(&g_now)); // MTasker letting the mthreads do their thing diff --git a/pdns/responsestats.cc b/pdns/responsestats.cc index 3fc4286ae4..b61aa03c13 100644 --- a/pdns/responsestats.cc +++ b/pdns/responsestats.cc @@ -8,9 +8,8 @@ #include "dnsparser.hh" -ResponseStats::ResponseStats() +ResponseStats::ResponseStats() : d_qtypecounters(65536) { - d_qtypecounters.resize(std::numeric_limits::max()+1); d_sizecounters.push_back(make_pair(20,0)); d_sizecounters.push_back(make_pair(40,0)); d_sizecounters.push_back(make_pair(60,0)); diff --git a/pdns/statbag.cc b/pdns/statbag.cc index 9d58e126cd..f6d07218b0 100644 --- a/pdns/statbag.cc +++ b/pdns/statbag.cc @@ -109,13 +109,13 @@ void StatBag::declare(const string &key, const string &descrip, StatBag::func_t } -void StatBag::set(const string &key, AtomicCounter::native_t value) +void StatBag::set(const string &key, unsigned long value) { exists(key); - *d_stats[key]=AtomicCounter(value); + d_stats[key]->store(value); } -AtomicCounter::native_t StatBag::read(const string &key) +unsigned long StatBag::read(const string &key) { exists(key); funcstats_t::const_iterator iter = d_funcstats.find(key); @@ -124,10 +124,10 @@ AtomicCounter::native_t StatBag::read(const string &key) return *d_stats[key]; } -AtomicCounter::native_t StatBag::readZero(const string &key) +unsigned long StatBag::readZero(const string &key) { exists(key); - AtomicCounter::native_t tmp=*d_stats[key]; + unsigned long tmp=*d_stats[key]; d_stats[key]=0; return tmp; } diff --git a/pdns/statbag.hh b/pdns/statbag.hh index 5adbce4544..f0b43866d7 100644 --- a/pdns/statbag.hh +++ b/pdns/statbag.hh @@ -116,9 +116,9 @@ public: void exists(const string &key); //!< call this function to throw an exception in case a key does not exist inline void deposit(const string &key, int value); //!< increment the statistics behind this key by value amount inline void inc(const string &key); //!< increase this key's value by one - void set(const string &key, AtomicCounter::native_t value); //!< set this key's value - AtomicCounter::native_t read(const string &key); //!< read the value behind this key - AtomicCounter::native_t readZero(const string &key); //!< read the value behind this key, and zero it afterwards + void set(const string &key, unsigned long value); //!< set this key's value + unsigned long read(const string &key); //!< read the value behind this key + unsigned long readZero(const string &key); //!< read the value behind this key, and zero it afterwards AtomicCounter *getPointer(const string &key); //!< get a direct pointer to the value behind a key. Use this for high performance increments string getValueStr(const string &key); //!< read a value behind a key, and return it as a string string getValueStrZero(const string &key); //!< read a value behind a key, and return it as a string, and zero afterwards -- 2.47.2