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 59040509f04b639304af85ed5f7904b8e9add9fd..82452968ada6e0aebb2dbb5d201b33e15e0e1d8a 100644 (file)
--- 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;