resolved: replace transaction list by hashmap

Right now we keep track of ongoing transactions in a linked listed for
each scope. Replace this by a hashmap that is indexed by the RR key.
Given that all ongoing transactions will be placed in pretty much the
same scopes usually this should optimize behaviour.

We used to require a list here, since we wanted to do "superset" query
checks, but this became obsolete since transactions are now single-key
instead of multi-key.

diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index 57a2c7d6c156425ab717233657b4f3f081d9fa87..336269ca8a6227bcc460cc8707fff2845327b6f6 100644 (file)
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -78,8 +78,7 @@ DnsScope* dns_scope_free(DnsScope *s) {
 
         dns_scope_llmnr_membership(s, false);
 
-        while ((t = s->transactions)) {
-
+        while ((t = hashmap_steal_first(s->transactions))) {
                 /* Abort the transaction, but make sure it is not
                  * freed while we still look at it */
 
@@ -90,6 +89,8 @@ DnsScope* dns_scope_free(DnsScope *s) {
                 dns_transaction_free(t);
         }
 
+        hashmap_free(s->transactions);
+
         while ((rr = ordered_hashmap_steal_first(s->conflict_queue)))
                 dns_resource_record_unref(rr);
 
@@ -623,24 +624,20 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key,
         assert(scope);
         assert(key);
 
-        /* Try to find an ongoing transaction that is a equal or a
-         * superset of the specified question */
-
-        LIST_FOREACH(transactions_by_scope, t, scope->transactions) {
-
-                /* Refuse reusing transactions that completed based on
-                 * cached data instead of a real packet, if that's
-                 * requested. */
-                if (!cache_ok &&
-                    IN_SET(t->state, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_FAILURE) &&
-                    !t->received)
-                        continue;
+        /* Try to find an ongoing transaction that is a equal to the
+         * specified question */
+        t = hashmap_get(scope->transactions, key);
+        if (!t)
+                return NULL;
 
-                if (dns_resource_key_equal(t->key, key) > 0)
-                        return t;
-        }
+        /* Refuse reusing transactions that completed based on cached
+         * data instead of a real packet, if that's requested. */
+        if (!cache_ok &&
+            IN_SET(t->state, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_FAILURE) &&
+            !t->received)
+                return NULL;
 
-        return NULL;
+        return t;
 }
 
 static int dns_scope_make_conflict_packet(

diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h
index 47f285db47fcf6beb6946521f78e8b15713a5f67..88fb8224e24c3f5ec9e6acbc3edf0a0eb8d808ba 100644 (file)
--- a/src/resolve/resolved-dns-scope.h
+++ b/src/resolve/resolved-dns-scope.h
@@ -60,7 +60,7 @@ struct DnsScope {
         usec_t resend_timeout;
         usec_t max_rtt;
 
-        LIST_HEAD(DnsTransaction, transactions);
+        Hashmap *transactions;
 
         LIST_FIELDS(DnsScope, scopes);
 };

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 81156dfa45a3e03eaeac69d0660747ef9eb5695e..608decd3ac8480a1087c1de3e5882d3fe6119034 100644 (file)
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -35,7 +35,6 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) {
 
         sd_event_source_unref(t->timeout_event_source);
 
-        dns_resource_key_unref(t->key);
         dns_packet_unref(t->sent);
         dns_packet_unref(t->received);
         dns_answer_unref(t->cached);
@@ -47,12 +46,14 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) {
         dns_stream_free(t->stream);
 
         if (t->scope) {
-                LIST_REMOVE(transactions_by_scope, t->scope->transactions, t);
+                hashmap_remove(t->scope->transactions, t->key);
 
                 if (t->id != 0)
                         hashmap_remove(t->scope->manager->dns_transactions, UINT_TO_PTR(t->id));
         }
 
+        dns_resource_key_unref(t->key);
+
         while ((q = set_steal_first(t->queries)))
                 set_remove(q->transactions, t);
         set_free(t->queries);
@@ -89,14 +90,18 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
         if (r < 0)
                 return r;
 
+        r = hashmap_ensure_allocated(&s->transactions, &dns_resource_key_hash_ops);
+        if (r < 0)
+                return r;
+
         t = new0(DnsTransaction, 1);
         if (!t)
                 return -ENOMEM;
 
         t->dns_fd = -1;
-
         t->key = dns_resource_key_ref(key);
 
+        /* Find a fresh, unused transaction id */
         do
                 random_bytes(&t->id, sizeof(t->id));
         while (t->id == 0 ||
@@ -108,7 +113,12 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
                 return r;
         }
 
-        LIST_PREPEND(transactions_by_scope, s->transactions, t);
+        r = hashmap_put(s->transactions, t->key, t);
+        if (r < 0) {
+                hashmap_remove(s->manager->dns_transactions, UINT_TO_PTR(t->id));
+                return r;
+        }
+
         t->scope = s;
 
         if (ret)