[svn] commit: r2224 - in /branches/trac192/src/lib: datasrc/cache.cc dns/question.h

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Jun 23 03:43:37 UTC 2010


Author: each
Date: Wed Jun 23 03:43:37 2010
New Revision: 2224

Log:
Addressing review comments: Removed QTuple class, use Question instead.
Added a comparison operator to Question so it could be used an an index
to std::map.

Modified:
    branches/trac192/src/lib/datasrc/cache.cc
    branches/trac192/src/lib/dns/question.h

Modified: branches/trac192/src/lib/datasrc/cache.cc
==============================================================================
--- branches/trac192/src/lib/datasrc/cache.cc (original)
+++ branches/trac192/src/lib/datasrc/cache.cc Wed Jun 23 03:43:37 2010
@@ -26,7 +26,17 @@
 /// and the query-response flags that were returned when the RRset
 /// was originally looked up in the low-level data source.
 class CacheEntry {
+private:
+    /// The copy constructor and the assignment operator are intentionally
+    /// defined as private.
+    //@{
+    CacheEntry(const CacheEntry& source);
+    CacheEntry& operator=(const CacheEntry& source);
+
 public:
+    CacheEntry(RRsetPtr r, uint32_t f) : rrset(r), flags(f) {};
+    //@}
+
     RRsetPtr rrset;
     uint32_t flags;
 };
@@ -35,9 +45,16 @@
 
 /// \brief A \c CacheNode is a node in the \c HotCache.
 class CacheNode {
+private:
+    /// \name Constructors, Assignment Operator and Destructor.
+    ///
+    /// Note: The copy constructor and the assignment operator are intentionally
+    /// defined as private.
+    //@{
+    CacheNode(const CacheNode& source);
+    CacheNode& operator=(const CacheNode& source);
+
 public:
-    /// \name Constructors and Destructor
-    //@{
     /// \brief Constructor for positive cache entry.
     ///
     /// \param rrset The \c RRset to cache.
@@ -71,12 +88,12 @@
     /// RRsetPtr for negative cache entries).
 
     /// \return \c RRsetPtr
-    RRsetPtr getRRset() const { return (entry_->rrset); }
+    RRsetPtr getRRset() const { return (entry->rrset); }
 
     /// \brief Returns the query response flags associated with the data.
     ///
     /// \return \c uint32_t
-    uint32_t getFlags() const { return (entry_->flags); }
+    uint32_t getFlags() const { return (entry->flags); }
 
     /// \brief Is this record still valid?
     ///
@@ -97,35 +114,10 @@
 
 private:
     // The cached RRset data
-    CacheEntryPtr entry_;
+    CacheEntryPtr entry;
 
     // When this record should be discarded
-    time_t expiry_;
-};
-
-/// \brief A \c QTuple is a simple object containing the standard query
-/// tuple: {name, class, type}.
-///
-/// Its purpose is to act as the key data format for the \c std::map
-/// in the \c HotCache class.
-///
-/// (Possible TODO item: using this in other parts of libdatasrc as well.)
-class QTuple {
-public:
-    QTuple(Name n, RRClass c, RRType t);
-
-    // A comparison operator is needed for this class to function
-    // as an index to std::map.
-    bool operator <(const QTuple& rhs) const {
-        return (qclass < rhs.qclass ||
-                (qclass == rhs.qclass &&
-                 (qtype < rhs.qtype ||
-                  (qtype == rhs.qtype && (qname < rhs.qname)))));
-    }
-
-    Name qname;
-    RRClass qclass;
-    RRType qtype;
+    time_t expiry;
 };
 
 /// This class abstracts the implementation details for the HotCache.
@@ -137,9 +129,9 @@
 /// used nodes will be removed from the tail of the list.
 ///
 /// A pointer to each cache node is also placed in a \c std::map object,
-/// keyed by \c QTuple.  This allows retrieval of data in (usually)
-/// logarithmic time.  (Possible TODO item: replace this with a hash
-/// table instead.)
+/// keyed by \c isc::dns::Question.  This allows retrieval of data in
+/// (usually) logarithmic time.  (Possible TODO item: replace this with a
+/// hash table instead.)
 class HotCacheImpl {
 public:
     HotCacheImpl(int slots);
@@ -158,7 +150,7 @@
     int count_;
 
     // Map from query tuple to cache node pointer, allowing faster retrieval
-    std::map<QTuple, CacheNodePtr> map_;
+    std::map<Question, CacheNodePtr> map_;
 
     // Move a node to the front of the LRU queue.
     void promote(CacheNodePtr node);
@@ -176,12 +168,9 @@
     qname(rrset->getName()), qclass(rrset->getClass()), qtype(rrset->getType())
 {
     const time_t now = time(NULL);
-    expiry_ = now + interval;
-
-    entry_ = CacheEntryPtr(new CacheEntry);
-    entry_->rrset = rrset;
-    entry_->flags = flags;
-
+    expiry = now + interval;
+
+    entry = CacheEntryPtr(new CacheEntry(rrset, flags));
     prev = CacheNodePtr();
     next = CacheNodePtr();
 }
@@ -194,12 +183,9 @@
     qname(name), qclass(rrclass), qtype(rrtype)
 {
     const time_t now = time(NULL);
-    expiry_ = now + interval;
-
-    entry_ = CacheEntryPtr(new CacheEntry);
-    entry_->rrset = RRsetPtr();
-    entry_->flags = flags;
-
+    expiry = now + interval;
+
+    entry = CacheEntryPtr(new CacheEntry(RRsetPtr(), flags));
     prev = CacheNodePtr();
     next = CacheNodePtr();
 }
@@ -210,11 +196,8 @@
 bool
 CacheNode::isValid() const {
     const time_t now = time(NULL);
-    return (now < expiry_);
-}
-
-// QTuple constructor
-QTuple::QTuple(Name n, RRClass c, RRType t) : qname(n), qclass(c), qtype(t) {} 
+    return (now < expiry);
+}
 
 // HotCache constructor
 HotCache::HotCache(const int slots) {
@@ -248,8 +231,8 @@
 HotCache::retrieve(Name n, RRClass c, RRType t,
                    RRsetPtr& rrset, uint32_t& flags)
 {
-    std::map<QTuple, CacheNodePtr>::const_iterator iter;
-    iter = impl_->map_.find(QTuple(n, c, t));
+    std::map<Question, CacheNodePtr>::const_iterator iter;
+    iter = impl_->map_.find(Question(n, c, t));
     if (iter == impl_->map_.end()) {
         return (false);
     }
@@ -306,8 +289,8 @@
 
 inline void
 HotCacheImpl::insert(const CacheNodePtr node) {
-    std::map<QTuple, CacheNodePtr>::const_iterator iter;
-    iter = map_.find(QTuple(node->qname, node->qclass, node->qtype));
+    std::map<Question, CacheNodePtr>::const_iterator iter;
+    iter = map_.find(Question(node->qname, node->qclass, node->qtype));
     if (iter != map_.end()) {
         CacheNodePtr old = iter->second;
         if (old && old->isValid()) {
@@ -325,7 +308,7 @@
         lru_tail_ = node;
     }
 
-    map_[QTuple(node->qname, node->qclass, node->qtype)] = node;
+    map_[Question(node->qname, node->qclass, node->qtype)] = node;
     ++count_;
 
     if (slots_ != 0 && count_ > slots_ && lru_tail_) {
@@ -381,7 +364,7 @@
         node->prev->next = node->next;
     }
 
-    map_.erase(QTuple(node->qname, node->qclass, node->qtype));
+    map_.erase(Question(node->qname, node->qclass, node->qtype));
     --count_;
 }
 

Modified: branches/trac192/src/lib/dns/question.h
==============================================================================
--- branches/trac192/src/lib/dns/question.h (original)
+++ branches/trac192/src/lib/dns/question.h Wed Jun 23 03:43:37 2010
@@ -229,6 +229,20 @@
     unsigned int toWire(OutputBuffer& buffer) const;
     //@}
 
+    ///
+    /// \name Comparison Operator
+    ///
+    //@{
+    /// A comparison operator is needed for this class so it can
+    /// function as an index to std::map.
+    bool operator <(const Question& rhs) const {
+        return (rrclass_ < rhs.rrclass_ ||
+                (rrclass_ == rhs.rrclass_ &&
+                 (rrtype_ < rhs.rrtype_ ||
+                  (rrtype_ == rhs.rrtype_ && (name_ < rhs.name_)))));
+    }
+    //@}
+
 private:
     Name name_;
     RRType rrtype_;




More information about the bind10-changes mailing list