diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index 8b2a049dec2d..9784752f88ea 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -1513,6 +1513,9 @@ void startDoResolve(void* arg) // NOLINT(readability-function-cognitive-complexi if (!ret.empty()) { #ifdef notyet + // As dedupping is relatively expensive do not dedup in general. We do have a few cases + // where we call dedup explicitly, e.g. when doing NAT64 or when adding NSEC records in + // doCNAMECacheCheck pdns::dedupRecords(ret); #endif pdns::orderAndShuffle(ret, false); diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc index b8d951aeb51c..3abfb5be5158 100644 --- a/pdns/recursordist/syncres.cc +++ b/pdns/recursordist/syncres.cc @@ -4448,6 +4448,7 @@ void SyncRes::sanitizeRecordsPass2(const std::string& prefix, LWResult& lwr, con lwr.d_records = std::move(vec); } #ifdef notyet + // As dedupping is relatively expensive and having dup records not really hurts as far as we have seen, do not dedup. if (auto count = pdns::dedupRecords(lwr.d_records); count > 0) { LOG(prefix << qname << ": Removed " << count << " duplicate records from response received from " << auth << endl); } diff --git a/pdns/shuffle.cc b/pdns/shuffle.cc index c9987b563342..9fb707b4ed5b 100644 --- a/pdns/shuffle.cc +++ b/pdns/shuffle.cc @@ -143,7 +143,7 @@ void pdns::orderAndShuffle(vector& rrs, bool includingAdditionals) unsigned int pdns::dedupRecords(vector& rrs) { - // This function tries to avoid unneccesary work + // This function tries to avoid unnecessary work // First a vector with zero or one element does not need dedupping if (rrs.size() <= 1) { return 0; @@ -174,7 +174,7 @@ unsigned int pdns::dedupRecords(vector& rrs) } // We avoid calling erase, as it calls a lot of move constructors. This can hurt, especially if - // you call it on a large vector muliple times. + // you call it on a large vector multiple times. // So we just take the elements that are unique std::vector ret; ret.reserve(rrs.size() - numDups); diff --git a/pdns/speedtest.cc b/pdns/speedtest.cc index c087bc69f794..3ac01f46234f 100644 --- a/pdns/speedtest.cc +++ b/pdns/speedtest.cc @@ -1186,18 +1186,7 @@ struct DedupRecordsTest { explicit DedupRecordsTest(size_t howmany, bool dedup, bool withdup = false) : d_howmany(howmany), d_dedup(dedup), d_withdup(withdup) { - } - - [[nodiscard]] string getName() const - { - return std::to_string(d_howmany) + " DedupRecords" + std::string(d_dedup ? "" : " (generate only)") + - std::string(d_withdup ? " (with dup)" : ""); - } - - void operator()() const - { - std::vector vec; - vec.reserve(d_howmany); + d_vec.reserve(d_howmany); std::string name("some.name.in.some.domain"); auto count = d_howmany; if (d_withdup) { @@ -1207,17 +1196,28 @@ struct DedupRecordsTest auto content = DNSRecordContent::make(QType::TXT, QClass::IN, "\"a text " + std::to_string(i) + "\""); DNSRecord rec(name, content, QType::TXT); if (i == 0 && d_withdup) { - vec.emplace_back(rec); + d_vec.emplace_back(rec); } - vec.emplace_back(std::move(rec)); + d_vec.emplace_back(std::move(rec)); } + } + + [[nodiscard]] string getName() const + { + return std::to_string(d_howmany) + " DedupRecords" + std::string(d_dedup ? "" : " (setup only)") + + std::string(d_withdup ? " (with dup)" : ""); + } + void operator()() const + { + auto vec{d_vec}; if (d_dedup) { pdns::dedupRecords(vec); } } private: + vector d_vec; size_t d_howmany; bool d_dedup; bool d_withdup;