From: Alessio Podda <alessio@isc.org>
Date: Thu, 30 Oct 2025 10:45:09 +0000 (+0100)
Subject: Skip buffer allocations if not logging
X-Git-Tag: v9.21.15~15^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a164492f3cfa4e6b247bc5274567dc46acb0cded;p=thirdparty%2Fbind9.git

Skip buffer allocations if not logging

Currently, during IXFR we allocate a 2KB buffer for IXFR change logging
regardless of the log level. This commit introduces an early check
on the log level in dns_diff_print to avoid this.

Results in a speedup from 28% in the test case from issue #5442.
---

diff --git a/lib/dns/diff.c b/lib/dns/diff.c
index 59040509f04..82452968ada 100644
--- a/lib/dns/diff.c
+++ b/lib/dns/diff.c
@@ -656,65 +656,73 @@ dns_diff_print(dns_diff_t *diff, FILE *file) {
 
 	REQUIRE(DNS_DIFF_VALID(diff));
 
-	mem = isc_mem_get(diff->mctx, size);
+	int required_log_level = ISC_LOG_DEBUG(7);
 
-	ISC_LIST_FOREACH(diff->tuples, t, link) {
-		isc_buffer_t buf;
-		isc_region_t r;
-
-		dns_rdatalist_t rdl;
-		dns_rdataset_t rds;
-		dns_rdata_t rd = DNS_RDATA_INIT;
-
-		diff_tuple_tordataset(t, &rd, &rdl, &rds);
-	again:
-		isc_buffer_init(&buf, mem, size);
-		result = dns_rdataset_totext(&rds, &t->name, false, false,
-					     &buf);
-
-		if (result == ISC_R_NOSPACE) {
-			isc_mem_put(diff->mctx, mem, size);
-			size += 1024;
-			mem = isc_mem_get(diff->mctx, size);
-			goto again;
-		}
+	/*
+	 * Logging requires allocating a buffer and some costly translation to
+	 * text. Avoid it if possible.
+	 */
+	if (isc_log_wouldlog(required_log_level) || file != NULL) {
+		mem = isc_mem_get(diff->mctx, size);
 
-		if (result != ISC_R_SUCCESS) {
-			goto cleanup;
-		}
-		/*
-		 * Get rid of final newline.
-		 */
-		INSIST(buf.used >= 1 &&
-		       ((char *)buf.base)[buf.used - 1] == '\n');
-		buf.used--;
-
-		isc_buffer_usedregion(&buf, &r);
-		switch (t->op) {
-		case DNS_DIFFOP_EXISTS:
-			op = "exists";
-			break;
-		case DNS_DIFFOP_ADD:
-			op = "add";
-			break;
-		case DNS_DIFFOP_DEL:
-			op = "del";
-			break;
-		case DNS_DIFFOP_ADDRESIGN:
-			op = "add re-sign";
-			break;
-		case DNS_DIFFOP_DELRESIGN:
-			op = "del re-sign";
-			break;
-		}
-		if (file != NULL) {
-			fprintf(file, "%s %.*s\n", op, (int)r.length,
-				(char *)r.base);
-		} else {
-			isc_log_write(DNS_LOGCATEGORY_GENERAL,
-				      DNS_LOGMODULE_DIFF, ISC_LOG_DEBUG(7),
-				      "%s %.*s", op, (int)r.length,
-				      (char *)r.base);
+		ISC_LIST_FOREACH(diff->tuples, t, link) {
+			isc_buffer_t buf;
+			isc_region_t r;
+
+			dns_rdatalist_t rdl;
+			dns_rdataset_t rds;
+			dns_rdata_t rd = DNS_RDATA_INIT;
+
+			diff_tuple_tordataset(t, &rd, &rdl, &rds);
+		again:
+			isc_buffer_init(&buf, mem, size);
+			result = dns_rdataset_totext(&rds, &t->name, false,
+						     false, &buf);
+
+			if (result == ISC_R_NOSPACE) {
+				isc_mem_put(diff->mctx, mem, size);
+				size += 1024;
+				mem = isc_mem_get(diff->mctx, size);
+				goto again;
+			}
+
+			if (result != ISC_R_SUCCESS) {
+				goto cleanup;
+			}
+			/*
+			 * Get rid of final newline.
+			 */
+			INSIST(buf.used >= 1 &&
+			       ((char *)buf.base)[buf.used - 1] == '\n');
+			buf.used--;
+
+			isc_buffer_usedregion(&buf, &r);
+			switch (t->op) {
+			case DNS_DIFFOP_EXISTS:
+				op = "exists";
+				break;
+			case DNS_DIFFOP_ADD:
+				op = "add";
+				break;
+			case DNS_DIFFOP_DEL:
+				op = "del";
+				break;
+			case DNS_DIFFOP_ADDRESIGN:
+				op = "add re-sign";
+				break;
+			case DNS_DIFFOP_DELRESIGN:
+				op = "del re-sign";
+				break;
+			}
+			if (file != NULL) {
+				fprintf(file, "%s %.*s\n", op, (int)r.length,
+					(char *)r.base);
+			} else {
+				isc_log_write(DNS_LOGCATEGORY_GENERAL,
+					      DNS_LOGMODULE_DIFF,
+					      required_log_level, "%s %.*s", op,
+					      (int)r.length, (char *)r.base);
+			}
 		}
 	}
 	result = ISC_R_SUCCESS;