resolved: keep addresses mapped to ::0 in a separate set

We'd store every 0.0.0.0 and ::0 entry as a structure without any addresses
allocated. This is a somewhat common use case, let's optimize it a bit.

This gives some memory savings and a bit faster response time too:
'time build/test-resolved-etc-hosts hosts' goes from 7.7s to 5.6s, and
memory use as reported by valgrind for ~10000 hosts is reduced
==18097==   total heap usage: 29,902 allocs, 29,902 frees, 2,136,437 bytes allocated
==18240==   total heap usage: 19,955 allocs, 19,955 frees, 1,556,021 bytes allocated

Also rename 'suppress' to 'found' (with reverse meaning). I think this makes
the intent clearer.

diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c
index 7b381f0561f757c6ec4264509a4f1fd49b44ead3..2adee0788977d6e3166962d97e33276928ad3519 100644 (file)
--- a/src/resolve/resolved-etc-hosts.c
+++ b/src/resolve/resolved-etc-hosts.c
@@ -26,6 +26,7 @@ static inline void etc_hosts_item_by_name_free(EtcHostsItemByName *item) {
 void etc_hosts_free(EtcHosts *hosts) {
         hosts->by_address = hashmap_free_with_destructor(hosts->by_address, etc_hosts_item_free);
         hosts->by_name = hashmap_free_with_destructor(hosts->by_name, etc_hosts_item_by_name_free);
+        hosts->no_address = set_free_free(hosts->no_address);
 }
 
 void manager_etc_hosts_flush(Manager *m) {
@@ -36,7 +37,7 @@ void manager_etc_hosts_flush(Manager *m) {
 static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) {
         _cleanup_free_ char *address_str = NULL;
         struct in_addr_data address = {};
-        bool suppressed = false;
+        bool found = false;
         EtcHostsItem *item;
         int r;
 
@@ -99,18 +100,32 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) {
                 if (r <= 0)
                         return log_error_errno(r, "Hostname %s is not valid, ignoring, in line /etc/hosts:%u.", name, nr);
 
-                if (is_localhost(name)) {
+                found = true;
+
+                if (is_localhost(name))
                         /* Suppress the "localhost" line that is often seen */
-                        suppressed = true;
                         continue;
-                }
 
-                if (item) {
-                        r = strv_extend(&item->names, name);
+                if (!item) {
+                        /* Optimize the case where we don't need to store any addresses, by storing
+                         * only the name in a dedicated Set instead of the hashmap */
+
+                        r = set_ensure_allocated(&hosts->no_address, &dns_name_hash_ops);
                         if (r < 0)
                                 return log_oom();
+
+                        r = set_put(hosts->no_address, name);
+                        if (r < 0)
+                                return r;
+
+                        TAKE_PTR(name);
+                        continue;
                 }
 
+                r = strv_extend(&item->names, name);
+                if (r < 0)
+                        return log_oom();
+
                 bn = hashmap_get(hosts->by_name, name);
                 if (!bn) {
                         r = hashmap_ensure_allocated(&hosts->by_name, &dns_name_hash_ops);
@@ -130,17 +145,13 @@ static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) {
                         bn->name = TAKE_PTR(name);
                 }
 
-                if (item) {
-                        if (!GREEDY_REALLOC(bn->addresses, bn->n_allocated, bn->n_addresses + 1))
-                                return log_oom();
-
-                        bn->addresses[bn->n_addresses++] = &item->address;
-                }
+                if (!GREEDY_REALLOC(bn->addresses, bn->n_allocated, bn->n_addresses + 1))
+                        return log_oom();
 
-                suppressed = true;
+                bn->addresses[bn->n_addresses++] = &item->address;
         }
 
-        if (!suppressed) {
+        if (!found) {
                 log_error("Line is missing any host names, in line /etc/hosts:%u.", nr);
                 return -EINVAL;
         }
@@ -310,12 +321,15 @@ int manager_etc_hosts_lookup(Manager *m, DnsQuestion* q, DnsAnswer **answer) {
         }
 
         bn = hashmap_get(m->etc_hosts.by_name, name);
-        if (!bn)
-                return 0;
-
-        r = dns_answer_reserve(answer, bn->n_addresses);
-        if (r < 0)
-                return r;
+        if (bn) {
+                r = dns_answer_reserve(answer, bn->n_addresses);
+                if (r < 0)
+                        return r;
+        } else {
+                /* Check if name was listed with no address. If yes, continue to return an answer. */
+                if (!set_contains(m->etc_hosts.no_address, name))
+                        return 0;
+        }
 
         DNS_QUESTION_FOREACH(t, q) {
                 if (!IN_SET(t->type, DNS_TYPE_A, DNS_TYPE_AAAA, DNS_TYPE_ANY))
@@ -338,7 +352,7 @@ int manager_etc_hosts_lookup(Manager *m, DnsQuestion* q, DnsAnswer **answer) {
                         break;
         }
 
-        for (i = 0; i < bn->n_addresses; i++) {
+        for (i = 0; bn && i < bn->n_addresses; i++) {
                 _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr = NULL;
 
                 if ((!found_a && bn->addresses[i]->family == AF_INET) ||

diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h
index c9cf2f0ee6abc504668a8e1c261b204c6fa1a5d2..79a473f57efb891affc20a8cf98a119038339a45 100644 (file)
--- a/src/resolve/resolved-manager.h
+++ b/src/resolve/resolved-manager.h
@@ -26,6 +26,7 @@ typedef struct Manager Manager;
 typedef struct EtcHosts {
         Hashmap *by_address;
         Hashmap *by_name;
+        Set *no_address;
 } EtcHosts;
 
 struct Manager {

diff --git a/src/resolve/test-resolved-etc-hosts.c b/src/resolve/test-resolved-etc-hosts.c
index 74a565fae71a671f1205f662a03cbbec2e140ad6..5b8f1e220e3b6f2c95029f88472d8cbe1f11f9f4 100644 (file)
--- a/src/resolve/test-resolved-etc-hosts.c
+++ b/src/resolve/test-resolved-etc-hosts.c
@@ -71,6 +71,11 @@ static void test_parse_etc_hosts(const char *fname) {
         assert_se(bn->addresses[0]->family == AF_INET6);
         assert_se(memcmp(&bn->addresses[0]->address.in6,
                          &(struct in6_addr) { .s6_addr = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5} }, 16 ) == 0);
+
+        assert_se( set_contains(hosts.no_address, "some.where"));
+        assert_se( set_contains(hosts.no_address, "some.other"));
+        assert_se( set_contains(hosts.no_address, "black.listed"));
+        assert_se(!set_contains(hosts.no_address, "foobar.foo.foo"));
 }
 
 int main(int argc, char **argv) {