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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2018-07-31 15:09:13 +02:00
parent 78fc21a11a
commit fd373593ba
3 changed files with 43 additions and 23 deletions

View File

@ -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,17 +100,31 @@ 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) {
/* 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;
}
if (item) {
r = strv_extend(&item->names, name);
if (r < 0)
return log_oom();
}
r = strv_extend(&item->names, name);
if (r < 0)
return log_oom();
bn = hashmap_get(hosts->by_name, name);
if (!bn) {
@ -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();
if (!GREEDY_REALLOC(bn->addresses, bn->n_allocated, bn->n_addresses + 1))
return log_oom();
bn->addresses[bn->n_addresses++] = &item->address;
}
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) ||

View File

@ -26,6 +26,7 @@ typedef struct Manager Manager;
typedef struct EtcHosts {
Hashmap *by_address;
Hashmap *by_name;
Set *no_address;
} EtcHosts;
struct Manager {

View File

@ -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) {