WIP: compact only if we deleted nodes

some calls to dns_qp_lookup() do not need partial matches, QP chains
or QP iterators. in these cases it's more efficient to use
dns_qp_getname().

lib/dns/include/dns/db.h

@@ -1076,15 +1076,19 @@ isc_result_t
 dns_db_createiterator(dns_db_t *db, unsigned int options,
 		      dns_dbiterator_t **iteratorp);
 /*%<
- * Create an iterator for version 'version' of 'db'.
+ * Create an iterator for 'db'.
  *
  * Notes:
  *
- * \li	One or more of the following options can be set.
+ * \li	One or more of the following options can be set:
+ *
  *	#DNS_DB_RELATIVENAMES
  *	#DNS_DB_NSEC3ONLY
  *	#DNS_DB_NONSEC3
  *
+ *	(Note that it is not mandatory to implement these flags;
+ *	some databases will ignore them.)
+ *
  * Requires:
  *
  * \li	'db' is a valid database.

lib/dns/include/dns/name.h

@@ -1339,4 +1339,14 @@ dns_name_isdnssvcb(const dns_name_t *name);
  * i.e. it starts with and optional _port label followed by a _dns label.
  */
 
+size_t
+dns_name_size(const dns_name_t *name);
+/*%<
+ * Return the amount of dynamically allocated memory associated with
+ * 'name' (which is 0 if 'name' is not dynamic).
+ *
+ * Requires:
+ * \li	'name' to be valid.
+ */
+
 ISC_LANG_ENDDECLS

lib/dns/include/dns/qp.h

@@ -536,9 +536,12 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name,
  *
  * If 'foundname' is not NULL, it will be updated to contain the name
  * that was found (if any). The return code, ISC_R_SUCCESS or
- * DNS_R_PARTIALMATCH, indicates whether the name found is name that
- * was requested, or an ancestor. If the result is ISC_R_NOTFOUND,
- * 'foundname' will not be updated.
+ * DNS_R_PARTIALMATCH, indicates whether the name found is the name
+ * that was requested, or an ancestor. If the result is ISC_R_NOTFOUND,
+ * 'foundname' will not be updated. (NOTE: the name will be constructed
+ * from the QP key of the found node, and this can be time-consuming.
+ * In performance-critical code, it is faster to store a copy of the
+ * name in the node data and use that instead of passing 'foundname'.)
  *
  * If 'chain' is not NULL, it is updated to contain a QP chain with
  * references to the populated nodes in the tree between the root and

lib/dns/include/dns/types.h

@@ -130,8 +130,6 @@ typedef struct dns_slabheader_proof dns_slabheader_proof_t;
 typedef struct dns_rbt		    dns_rbt_t;
 typedef struct dns_rbtdb	    dns_rbtdb_t;
 typedef struct dns_rbtdb_version    dns_rbtdb_version_t;
-typedef struct dns_qpdb		    dns_qpdb_t;
-typedef struct dns_qpdb_version	    dns_qpdb_version_t;
 typedef struct dns_rbtnode	    dns_rbtnode_t;
 typedef ISC_LIST(dns_rbtnode_t) dns_rbtnodelist_t;
 typedef uint16_t		  dns_rcode_t;
@@ -148,13 +146,11 @@ typedef struct dns_request	dns_request_t;
 typedef struct dns_requestmgr	dns_requestmgr_t;
 typedef struct dns_resolver	dns_resolver_t;
 typedef struct dns_rpsdb	dns_rpsdb_t;
-typedef struct dns_qpdata	dns_qpdata_t;
-typedef ISC_LIST(dns_qpdata_t) dns_qpdatalist_t;
-typedef struct dns_qpnode     dns_qpnode_t;
-typedef uint8_t		      dns_secalg_t;
-typedef uint8_t		      dns_secproto_t;
-typedef struct dns_signature  dns_signature_t;
-typedef struct dns_slabheader dns_slabheader_t;
+typedef struct dns_qpnode	dns_qpnode_t;
+typedef uint8_t			dns_secalg_t;
+typedef uint8_t			dns_secproto_t;
+typedef struct dns_signature	dns_signature_t;
+typedef struct dns_slabheader	dns_slabheader_t;
 typedef ISC_LIST(dns_slabheader_t) dns_slabheaderlist_t;
 typedef struct dns_sortlist_arg	  dns_sortlist_arg_t;
 typedef struct dns_ssurule	  dns_ssurule_t;

lib/dns/name.c

@@ -1840,6 +1840,24 @@ dns_name_free(dns_name_t *name, isc_mem_t *mctx) {
 	dns_name_invalidate(name);
 }
 
+size_t
+dns_name_size(const dns_name_t *name) {
+	size_t size;
+
+	REQUIRE(DNS_NAME_VALID(name));
+
+	if (!name->attributes.dynamic) {
+		return (0);
+	}
+
+	size = name->length;
+	if (name->attributes.dynoffsets) {
+		size += name->labels;
+	}
+
+	return (size);
+}
+
 isc_result_t
 dns_name_digest(const dns_name_t *name, dns_digestfunc_t digest, void *arg) {
 	dns_name_t downname;

lib/dns/qp.c

@@ -1059,7 +1059,7 @@ dns_qpmulti_memusage(dns_qpmulti_t *multi) {
 
 	dns_qp_memusage_t memusage = dns_qp_memusage(qp);
 
-	if (qp->transaction_mode == QP_UPDATE) {
+	if (qp->transaction_mode == QP_UPDATE && qp->usage != NULL) {
 		memusage.bytes -= QP_CHUNK_BYTES;
 		memusage.bytes += qp->usage[qp->bump].used *
 				  sizeof(dns_qpnode_t);

lib/dns/qpzone.c

@@ -235,9 +235,9 @@ static dns_dbmethods_t qpdb_zonemethods;
 #define qpdata_attach(ptr, ptrp) \
 	qpdata__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
 #define qpdata_detach(ptrp) qpdata__detach(ptrp, __func__, __FILE__, __LINE__)
-ISC_REFCOUNT_TRACE_DECL(qpdata);
+ISC_REFCOUNT_STATIC_TRACE_DECL(qpdata);
 #else
-ISC_REFCOUNT_DECL(qpdata);
+ISC_REFCOUNT_STATIC_DECL(qpdata);
 #endif
 
 /* QP trie methods */
@@ -3431,8 +3431,11 @@ find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
 	/*
 	 * Search down from the root of the tree.
 	 */
-	result = dns_qp_lookup(&search.qpr, name, foundname, &search.iter,
+	result = dns_qp_lookup(&search.qpr, name, NULL, &search.iter,
 			       &search.chain, (void **)&node, NULL);
+	if (result != ISC_R_NOTFOUND) {
+		dns_name_copy(&node->name, foundname);
+	}
 
 	/*
 	 * Check the QP chain to see if there's a node above us with a
@@ -3453,10 +3456,11 @@ find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
 		tresult = check_zonecut(n, &search DNS__DB_FLARG_PASS);
 		if (tresult != DNS_R_CONTINUE) {
 			result = tresult;
-			dns_qpchain_node(&search.chain, i, foundname, NULL,
-					 NULL);
 			search.chain.len = i - 1;
 			node = n;
+			if (foundname != NULL) {
+				dns_name_copy(&node->name, foundname);
+			}
 		}
 	}
 
@@ -4462,10 +4466,9 @@ dbiterator_seek(dns_dbiterator_t *iterator,
 				       qpdbiter->current, NULL,
 				       (void **)&qpdbiter->node, NULL);
 		if (result == DNS_R_PARTIALMATCH) {
-			dns_qpdata_t *node = NULL;
 			tresult = dns_qp_lookup(qpdbiter->nsnap, name, NULL,
 						&qpdbiter->nsec3iter, NULL,
-						(void **)&node, NULL);
+						NULL, NULL);
 			if (tresult == ISC_R_SUCCESS) {
 				qpdbiter->current = &qpdbiter->nsec3iter;
 				result = tresult;
@@ -5390,9 +5393,9 @@ destroy_qpdata(qpdata_t *node) {
 }
 
 #if DNS_DB_NODETRACE
-ISC_REFCOUNT_TRACE_IMPL(qpdata, destroy_qpdata);
+ISC_REFCOUNT_STATIC_TRACE_IMPL(qpdata, destroy_qpdata);
 #else
-ISC_REFCOUNT_IMPL(qpdata, destroy_qpdata);
+ISC_REFCOUNT_STATIC_IMPL(qpdata, destroy_qpdata);
 #endif
 
 static void

lib/isc/include/isc/refcount.h

@@ -119,19 +119,26 @@ typedef atomic_uint_fast32_t isc_refcount_t;
 		ISC_INSIST(_refs > 0);                                \
 	} while (0)
 
-#define ISC_REFCOUNT_TRACE_DECL(name)                                         \
-	name##_t *name##__ref(name##_t *ptr, const char *func,                \
-			      const char *file, unsigned int line);           \
-	void name##__unref(name##_t *ptr, const char *func, const char *file, \
-			   unsigned int line);                                \
-	void name##__attach(name##_t *ptr, name##_t **ptrp, const char *func, \
-			    const char *file, unsigned int line);             \
-	void name##__detach(name##_t **ptrp, const char *func,                \
-			    const char *file, unsigned int line)
+#define ISC__REFCOUNT_TRACE_DECL(name, stat)                               \
+	stat name##_t *name##__ref(name##_t *ptr, const char *func,        \
+				   const char *file, unsigned int line);   \
+	stat void      name##__unref(name##_t *ptr, const char *func,      \
+				     const char *file, unsigned int line); \
+	stat void      name##__attach(name##_t *ptr, name##_t **ptrp,      \
+				      const char *func, const char *file,  \
+				      unsigned int line);                  \
+	stat	       name##__detach(name##_t **ptrp, const char *func,   \
+				      const char *file, unsigned int line)
 
-#define ISC_REFCOUNT_TRACE_IMPL(name, destroy)                                \
-	name##_t *name##__ref(name##_t *ptr, const char *func,                \
-			      const char *file, unsigned int line) {          \
+#define ISC_REFCOUNT_BLANK
+#define ISC_REFCOUNT_TRACE_DECL(name) \
+	ISC__REFCOUNT_TRACE_DECL(name, ISC_REFCOUNT_BLANK)
+#define ISC_REFCOUNT_STATIC_TRACE_DECL(name) \
+	ISC__REFCOUNT_TRACE_DECL(name, static inline)
+
+#define ISC__REFCOUNT_TRACE_IMPL(name, destroy, stat)                         \
+	stat name##_t *name##__ref(name##_t *ptr, const char *func,           \
+				   const char *file, unsigned int line) {     \
 		REQUIRE(ptr != NULL);                                         \
 		uint_fast32_t refs =                                          \
 			isc_refcount_increment(&ptr->references) + 1;         \
@@ -141,8 +148,8 @@ typedef atomic_uint_fast32_t isc_refcount_t;
 		return (ptr);                                                 \
 	}                                                                     \
                                                                               \
-	void name##__unref(name##_t *ptr, const char *func, const char *file, \
-			   unsigned int line) {                               \
+	stat void name##__unref(name##_t *ptr, const char *func,              \
+				const char *file, unsigned int line) {        \
 		REQUIRE(ptr != NULL);                                         \
 		uint_fast32_t refs =                                          \
 			isc_refcount_decrement(&ptr->references) - 1;         \
@@ -154,8 +161,9 @@ typedef atomic_uint_fast32_t isc_refcount_t;
 			"%s:%s:%s:%u:t%u:%p->references = %" PRIuFAST32 "\n", \
 			__func__, func, file, line, isc_tid(), ptr, refs);    \
 	}                                                                     \
-	void name##__attach(name##_t *ptr, name##_t **ptrp, const char *func, \
-			    const char *file, unsigned int line) {            \
+	stat void name##__attach(name##_t *ptr, name##_t **ptrp,              \
+				 const char *func, const char *file,          \
+				 unsigned int line) {                         \
 		REQUIRE(ptrp != NULL && *ptrp == NULL);                       \
 		uint_fast32_t refs =                                          \
 			isc_refcount_increment(&ptr->references) + 1;         \
@@ -165,8 +173,8 @@ typedef atomic_uint_fast32_t isc_refcount_t;
 		*ptrp = ptr;                                                  \
 	}                                                                     \
                                                                               \
-	void name##__detach(name##_t **ptrp, const char *func,                \
-			    const char *file, unsigned int line) {            \
+	stat void name##__detach(name##_t **ptrp, const char *func,           \
+				 const char *file, unsigned int line) {       \
 		REQUIRE(ptrp != NULL && *ptrp != NULL);                       \
 		name##_t *ptr = *ptrp;                                        \
 		*ptrp = NULL;                                                 \
@@ -181,37 +189,51 @@ typedef atomic_uint_fast32_t isc_refcount_t;
 			__func__, func, file, line, isc_tid(), ptr, refs);    \
 	}
 
-#define ISC_REFCOUNT_DECL(name)                                  \
-	name##_t *name##_ref(name##_t *ptr);                     \
-	void	  name##_unref(name##_t *ptr);                   \
-	void	  name##_attach(name##_t *ptr, name##_t **ptrp); \
-	void	  name##_detach(name##_t **ptrp)
+#define ISC_REFCOUNT_TRACE_IMPL(name, destroy) \
+	ISC__REFCOUNT_TRACE_DECL(name, destroy, ISC_REFCOUNT_BLANK)
+#define ISC_REFCOUNT_STATIC_TRACE_IMPL(name, destroy) \
+	ISC__REFCOUNT_TRACE_DECL(name, destroy, static inline)
 
-#define ISC_REFCOUNT_IMPL(name, destroy)                             \
-	name##_t *name##_ref(name##_t *ptr) {                        \
+#define ISC__REFCOUNT_DECL(name, stat)                                      \
+	stat name##_t *name##_ref(name##_t *ptr) __attribute__((unused));   \
+	stat void      name##_unref(name##_t *ptr) __attribute__((unused)); \
+	stat void      name##_attach(name##_t *ptr, name##_t **ptrp)        \
+		__attribute__((unused));                                    \
+	stat void name##_detach(name##_t **ptrp) __attribute__((unused))
+
+#define ISC_REFCOUNT_DECL(name)	       ISC__REFCOUNT_DECL(name, ISC_REFCOUNT_BLANK)
+#define ISC_REFCOUNT_STATIC_DECL(name) ISC__REFCOUNT_DECL(name, static inline)
+
+#define ISC__REFCOUNT_IMPL(name, destroy, stat)                      \
+	stat name##_t *name##_ref(name##_t *ptr) {                   \
 		REQUIRE(ptr != NULL);                                \
 		isc_refcount_increment(&ptr->references);            \
 		return (ptr);                                        \
 	}                                                            \
                                                                      \
-	void name##_unref(name##_t *ptr) {                           \
+	stat void name##_unref(name##_t *ptr) {                      \
 		REQUIRE(ptr != NULL);                                \
 		if (isc_refcount_decrement(&ptr->references) == 1) { \
 			isc_refcount_destroy(&ptr->references);      \
 			destroy(ptr);                                \
 		}                                                    \
 	}                                                            \
-	void name##_attach(name##_t *ptr, name##_t **ptrp) {         \
+	stat void name##_attach(name##_t *ptr, name##_t **ptrp) {    \
 		REQUIRE(ptrp != NULL && *ptrp == NULL);              \
 		name##_ref(ptr);                                     \
 		*ptrp = ptr;                                         \
 	}                                                            \
                                                                      \
-	void name##_detach(name##_t **ptrp) {                        \
+	stat void name##_detach(name##_t **ptrp) {                   \
 		REQUIRE(ptrp != NULL && *ptrp != NULL);              \
 		name##_t *ptr = *ptrp;                               \
 		*ptrp = NULL;                                        \
 		name##_unref(ptr);                                   \
 	}
 
+#define ISC_REFCOUNT_IMPL(name, destroy) \
+	ISC__REFCOUNT_IMPL(name, destroy, ISC_REFCOUNT_BLANK)
+#define ISC_REFCOUNT_STATIC_IMPL(name, destroy) \
+	ISC__REFCOUNT_IMPL(name, destroy, static inline)
+
 ISC_LANG_ENDDECLS

tests/dns/qpdb_test.c

@@ -140,12 +140,14 @@ ISC_RUN_TEST_IMPL(overmempurge_bigrdata) {
 
 	/*
 	 * Then try to add the same number of entries, each has very large data.
-	 * 'overmem purge' should keep the total cache size from not exceeding
+	 * 'overmem purge' should keep the total cache size from exceeding
 	 * the 'hiwater' mark too much. So we should be able to assume the
 	 * cache size doesn't reach the "max".
 	 */
 	while (i-- > 0) {
 		overmempurge_addrdataset(db, now, i, 50054, 65535, false);
+		print_message("# inuse: %zd max: %zd\n", isc_mem_inuse(mctx2),
+			      maxcache);
 		assert_true(isc_mem_inuse(mctx2) < maxcache);
 	}
 
@@ -191,6 +193,8 @@ ISC_RUN_TEST_IMPL(overmempurge_longname) {
 	 */
 	while (i-- > 0) {
 		overmempurge_addrdataset(db, now, i, 50054, 0, true);
+		print_message("# inuse: %zd max: %zd\n", isc_mem_inuse(mctx2),
+			      maxcache);
 		assert_true(isc_mem_inuse(mctx2) < maxcache);
 	}