From: Remi Gacogne Date: Mon, 10 May 2021 13:53:56 +0000 (+0200) Subject: dnsdist: Reorganize the IDState and Rings fields X-Git-Tag: auth-4.5.0-alpha1~10^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5091f6c04b4bc270b2400e1d456b7a1459c64daa;p=thirdparty%2Fpdns.git dnsdist: Reorganize the IDState and Rings fields Reducing the space lost to padding and thus the memory usage. This change saves 1 MB of memory per downstream server in the default configuration, and around 8 bytes per entry in the ring buffer. --- diff --git a/pdns/dnsdist-rings.hh b/pdns/dnsdist-rings.hh index 015bd619d4..d987880668 100644 --- a/pdns/dnsdist-rings.hh +++ b/pdns/dnsdist-rings.hh @@ -36,23 +36,23 @@ struct Rings { struct Query { - struct timespec when; ComboAddress requestor; DNSName name; + struct timespec when; + struct dnsheader dh; uint16_t size; uint16_t qtype; - struct dnsheader dh; }; struct Response { - struct timespec when; ComboAddress requestor; + ComboAddress ds; // who handled it DNSName name; - uint16_t qtype; + struct timespec when; + struct dnsheader dh; unsigned int usec; unsigned int size; - struct dnsheader dh; - ComboAddress ds; // who handled it + uint16_t qtype; }; struct Shard @@ -216,7 +216,7 @@ private: if (!shard->queryRing.full()) { d_nbQueryEntries++; } - shard->queryRing.push_back({when, requestor, name, size, qtype, dh}); + shard->queryRing.push_back({requestor, name, when, dh, size, qtype}); } void insertResponseLocked(std::unique_ptr& shard, const struct timespec& when, const ComboAddress& requestor, const DNSName& name, uint16_t qtype, unsigned int usec, unsigned int size, const struct dnsheader& dh, const ComboAddress& backend) @@ -224,7 +224,7 @@ private: if (!shard->respRing.full()) { d_nbResponseEntries++; } - shard->respRing.push_back({when, requestor, name, qtype, usec, size, dh, backend}); + shard->respRing.push_back({requestor, backend, name, when, dh, usec, size, qtype}); } std::atomic d_nbQueryEntries; diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index e2448fab46..6313c4ea1f 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -557,9 +557,9 @@ struct ClientState; struct IDState { - IDState(): sentTime(true), delayMsec(0), tempFailureTTL(boost::none) { origDest.sin4.sin_family = 0;} + IDState(): sentTime(true), tempFailureTTL(boost::none) { origDest.sin4.sin_family = 0;} IDState(const IDState& orig) = delete; - IDState(IDState&& rhs): origRemote(rhs.origRemote), origDest(rhs.origDest), sentTime(rhs.sentTime), qname(std::move(rhs.qname)), dnsCryptQuery(std::move(rhs.dnsCryptQuery)), subnet(rhs.subnet), packetCache(std::move(rhs.packetCache)), qTag(std::move(rhs.qTag)), cs(rhs.cs), du(std::move(rhs.du)), cacheKey(rhs.cacheKey), cacheKeyNoECS(rhs.cacheKeyNoECS), qtype(rhs.qtype), qclass(rhs.qclass), origID(rhs.origID), origFlags(rhs.origFlags), origFD(rhs.origFD), delayMsec(rhs.delayMsec), tempFailureTTL(rhs.tempFailureTTL), ednsAdded(rhs.ednsAdded), ecsAdded(rhs.ecsAdded), skipCache(rhs.skipCache), destHarvested(rhs.destHarvested), dnssecOK(rhs.dnssecOK), useZeroScope(rhs.useZeroScope) + IDState(IDState&& rhs): subnet(rhs.subnet), origRemote(rhs.origRemote), origDest(rhs.origDest), hopRemote(rhs.hopRemote), hopLocal(rhs.hopLocal), qname(std::move(rhs.qname)), sentTime(rhs.sentTime), dnsCryptQuery(std::move(rhs.dnsCryptQuery)), packetCache(std::move(rhs.packetCache)), qTag(std::move(rhs.qTag)), tempFailureTTL(rhs.tempFailureTTL), cs(rhs.cs), du(std::move(rhs.du)), cacheKey(rhs.cacheKey), cacheKeyNoECS(rhs.cacheKeyNoECS), origFD(rhs.origFD), delayMsec(rhs.delayMsec), qtype(rhs.qtype), qclass(rhs.qclass), origID(rhs.origID), origFlags(rhs.origFlags), ednsAdded(rhs.ednsAdded), ecsAdded(rhs.ecsAdded), skipCache(rhs.skipCache), destHarvested(rhs.destHarvested), dnssecOK(rhs.dnssecOK), useZeroScope(rhs.useZeroScope) { if (rhs.isInUse()) { throw std::runtime_error("Trying to move an in-use IDState"); @@ -583,37 +583,39 @@ struct IDState throw std::runtime_error("Trying to move an in-use IDState"); } + subnet = std::move(rhs.subnet); origRemote = rhs.origRemote; origDest = rhs.origDest; - sentTime = rhs.sentTime; + hopRemote = rhs.hopRemote; + hopLocal = rhs.hopLocal; qname = std::move(rhs.qname); + sentTime = rhs.sentTime; dnsCryptQuery = std::move(rhs.dnsCryptQuery); - subnet = rhs.subnet; packetCache = std::move(rhs.packetCache); qTag = std::move(rhs.qTag); + tempFailureTTL = std::move(rhs.tempFailureTTL); cs = rhs.cs; du = std::move(rhs.du); cacheKey = rhs.cacheKey; cacheKeyNoECS = rhs.cacheKeyNoECS; + origFD = rhs.origFD; + delayMsec = rhs.delayMsec; +#ifdef __SANITIZE_THREAD__ + age.store(rhs.age.load()); +#else + age = rhs.age; +#endif qtype = rhs.qtype; qclass = rhs.qclass; origID = rhs.origID; origFlags = rhs.origFlags; - origFD = rhs.origFD; - delayMsec = rhs.delayMsec; - tempFailureTTL = rhs.tempFailureTTL; + uniqueId = std::move(rhs.uniqueId); ednsAdded = rhs.ednsAdded; ecsAdded = rhs.ecsAdded; skipCache = rhs.skipCache; destHarvested = rhs.destHarvested; dnssecOK = rhs.dnssecOK; useZeroScope = rhs.useZeroScope; -#ifdef __SANITIZE_THREAD__ - age.store(rhs.age.load()); -#else - age = rhs.age; -#endif - uniqueId = std::move(rhs.uniqueId); return *this; } @@ -637,14 +639,14 @@ struct IDState return usageIndicator.compare_exchange_strong(expectedUsageIndicator, unusedIndicator); } - /* mark as unused no matter what, return true if the state was in use before */ + /* mark as used no matter what, return true if the state was in use before */ bool markAsUsed() { auto currentGeneration = generation++; return markAsUsed(currentGeneration); } - /* mark as unused no matter what, return true if the state was in use before */ + /* mark as used no matter what, return true if the state was in use before */ bool markAsUsed(int64_t currentGeneration) { int64_t oldUsage = usageIndicator.exchange(currentGeneration); @@ -684,35 +686,35 @@ struct IDState wrapping around if necessary, and we set an atomic signed 64-bit value, so that we still have -1 when the state is unused and the value of our counter otherwise. */ - std::atomic usageIndicator{unusedIndicator}; // set to unusedIndicator to indicate this state is empty // 8 - std::atomic generation{0}; // increased every time a state is used, to be able to detect an ABA issue // 4 + boost::optional subnet{boost::none}; // 40 ComboAddress origRemote; // 28 ComboAddress origDest; // 28 ComboAddress hopRemote; ComboAddress hopLocal; + DNSName qname; // 24 StopWatch sentTime; // 16 - DNSName qname; // 80 - std::shared_ptr dnsCryptQuery{nullptr}; - boost::optional uniqueId; - boost::optional subnet{boost::none}; - std::shared_ptr packetCache{nullptr}; - std::shared_ptr qTag{nullptr}; - const ClientState* cs{nullptr}; - DOHUnit* du{nullptr}; + std::shared_ptr dnsCryptQuery{nullptr}; // 16 + std::shared_ptr packetCache{nullptr}; // 16 + std::shared_ptr qTag{nullptr}; // 16 + boost::optional tempFailureTTL; // 8 + const ClientState* cs{nullptr}; // 8 + DOHUnit* du{nullptr}; // 8 + std::atomic usageIndicator{unusedIndicator}; // set to unusedIndicator to indicate this state is empty // 8 + std::atomic generation{0}; // increased every time a state is used, to be able to detect an ABA issue // 4 uint32_t cacheKey{0}; // 4 uint32_t cacheKeyNoECS{0}; // 4 + int origFD{-1}; // 4 + int delayMsec{0}; #ifdef __SANITIZE_THREAD__ std::atomic age{0}; #else - uint16_t age{0}; // 4 + uint16_t age{0}; // 2 #endif uint16_t qtype{0}; // 2 uint16_t qclass{0}; // 2 uint16_t origID{0}; // 2 uint16_t origFlags{0}; // 2 - int origFD{-1}; - int delayMsec{0}; - boost::optional tempFailureTTL; + boost::optional uniqueId{boost::none}; // 17 (placed here to reduce the space lost to padding) bool ednsAdded{false}; bool ecsAdded{false}; bool skipCache{false}; diff --git a/pdns/dnsdistdist/test-dnsdistrings_cc.cc b/pdns/dnsdistdist/test-dnsdistrings_cc.cc index 09181c221f..fee1f51dd9 100644 --- a/pdns/dnsdistdist/test-dnsdistrings_cc.cc +++ b/pdns/dnsdistdist/test-dnsdistrings_cc.cc @@ -197,8 +197,8 @@ BOOST_AUTO_TEST_CASE(test_Rings_Threaded) { uint16_t size = 42; Rings rings(numberOfEntries, numberOfShards, lockAttempts, true); - Rings::Query query({now, requestor, qname, size, qtype, dh}); - Rings::Response response({now, requestor, qname, qtype, latency, size, dh, server}); + Rings::Query query({requestor, qname, now, dh, size, qtype}); + Rings::Response response({requestor, server, qname, now, dh, latency, size, qtype}); std::atomic done(false); std::vector writerThreads;