gossipd: use htable instead of simple array for node's channels.

For giant nodes, it seems we spend a lot of time memmoving this array.
Normally we'd go for a linked list, but that's actually hard: each
channel has two nodes, so needs two embedded list pointers, and when
iterating there's no good way to figure out which embedded pointer
we'd be using.

So we (ab)use htable; we don't really need an index, but it's good for
cache-friendly iteration (our main operation).  We can actually change
to a hybrid later to avoid the extra allocation for small nodes.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

gossipd/gossipd.c

@@ -175,8 +175,12 @@ struct peer {
 static void peer_disable_channels(struct daemon *daemon, struct node *node)
 {
 	/* If this peer had a channel with us, mark it disabled. */
-	for (size_t i = 0; i < tal_count(node->chans); i++) {
-		struct chan *c = node->chans[i];
+	struct chan_map_iter i;
+	struct chan *c;
+
+	for (c = chan_map_first(&node->chans, &i);
+	     c;
+	     c = chan_map_next(&node->chans, &i)) {
 		if (pubkey_eq(&other_node(node, c)->id, &daemon->id))
 			c->local_disabled = true;
 	}
@@ -1798,8 +1802,13 @@ static void gossip_refresh_network(struct daemon *daemon)
 	if (n) {
 		/* Iterate through all outgoing connection and check whether
 		 * it's time to re-announce */
-		for (size_t i = 0; i < tal_count(n->chans); i++) {
-			struct half_chan *hc = half_chan_from(n, n->chans[i]);
+		struct chan_map_iter i;
+		struct chan *c;
+
+		for (c = chan_map_first(&n->chans, &i);
+		     c;
+		     c = chan_map_next(&n->chans, &i)) {
+			struct half_chan *hc = half_chan_from(n, c);
 
 			if (!is_halfchan_defined(hc)) {
 				/* Connection is not announced yet, so don't even
@@ -1817,7 +1826,7 @@ static void gossip_refresh_network(struct daemon *daemon)
 				continue;
 			}
 
-			gossip_send_keepalive_update(daemon, n->chans[i], hc);
+			gossip_send_keepalive_update(daemon, c, hc);
 		}
 	}
 
@@ -1830,14 +1839,18 @@ static void gossip_refresh_network(struct daemon *daemon)
 static void gossip_disable_local_channels(struct daemon *daemon)
 {
 	struct node *local_node = get_node(daemon->rstate, &daemon->id);
+	struct chan_map_iter i;
+	struct chan *c;
 
 	/* We don't have a local_node, so we don't have any channels yet
 	 * either */
 	if (!local_node)
 		return;
 
-	for (size_t i = 0; i < tal_count(local_node->chans); i++)
-		local_node->chans[i]->local_disabled = true;
+	for (c = chan_map_first(&local_node->chans, &i);
+	     c;
+	     c = chan_map_next(&local_node->chans, &i))
+		c->local_disabled = true;
 }
 
 /*~ Parse init message from lightningd: starts the daemon properly. */
@@ -2013,11 +2026,16 @@ static struct io_plan *getchannels_req(struct io_conn *conn,
 	} else if (source) {
 		struct node *s = get_node(daemon->rstate, source);
 		if (s) {
-			for (size_t i = 0; i < tal_count(s->chans); i++)
+			struct chan_map_iter i;
+			struct chan *c;
+
+			for (c = chan_map_first(&s->chans, &i);
+			     c;
+			     c = chan_map_next(&s->chans, &i)) {
 				append_half_channel(&entries,
-						    s->chans[i],
-						    !half_chan_to(s,
-								  s->chans[i]));
+						    c,
+						    !half_chan_to(s, c));
+			}
 		}
 	} else {
 		u64 idx;
@@ -2152,10 +2170,15 @@ out:
 static bool node_has_public_channels(const struct node *peer,
 				     const struct chan *exclude)
 {
-	for (size_t i = 0; i < tal_count(peer->chans); i++) {
-		if (peer->chans[i] == exclude)
+	struct chan_map_iter i;
+	struct chan *c;
+
+	for (c = chan_map_first(&peer->chans, &i);
+	     c;
+	     c = chan_map_next(&peer->chans, &i)) {
+		if (c == exclude)
 			continue;
-		if (is_chan_public(peer->chans[i]))
+		if (is_chan_public(c))
 			return true;
 	}
 	return false;
@@ -2200,8 +2223,12 @@ static struct io_plan *get_incoming_channels(struct io_conn *conn,
 
 	node = get_node(daemon->rstate, &daemon->rstate->local_id);
 	if (node) {
-		for (size_t i = 0; i < tal_count(node->chans); i++) {
-			const struct chan *c = node->chans[i];
+		struct chan_map_iter i;
+		struct chan *c;
+
+		for (c = chan_map_first(&node->chans, &i);
+		     c;
+		     c = chan_map_next(&node->chans, &i)) {
 			const struct half_chan *hc;
 			struct route_info ri;
 

gossipd/routing.c

@@ -120,11 +120,14 @@ bool node_map_node_eq(const struct node *n, const struct pubkey *key)
 
 static void destroy_node(struct node *node, struct routing_state *rstate)
 {
+	struct chan_map_iter i;
+	struct chan *c;
 	node_map_del(rstate->nodes, node);
 
-	/* These remove themselves from the array. */
-	while (tal_count(node->chans))
-		tal_free(node->chans[0]);
+	/* These remove themselves from the map. */
+	while ((c = chan_map_first(&node->chans, &i)) != NULL)
+		tal_free(c);
+	chan_map_clear(&node->chans);
 }
 
 struct node *get_node(struct routing_state *rstate, const struct pubkey *id)
@@ -141,7 +144,7 @@ static struct node *new_node(struct routing_state *rstate,
 
 	n = tal(rstate, struct node);
 	n->id = *id;
-	n->chans = tal_arr(n, struct chan *, 0);
+	chan_map_init(&n->chans);
 	n->globalfeatures = NULL;
 	n->node_announcement = NULL;
 	n->node_announcement_index = 0;
@@ -156,9 +159,15 @@ static struct node *new_node(struct routing_state *rstate,
 /* We've received a channel_announce for a channel attached to this node */
 static bool node_has_public_channels(struct node *node)
 {
-	for (size_t i = 0; i < tal_count(node->chans); i++)
-		if (is_chan_public(node->chans[i]))
+	struct chan_map_iter i;
+	struct chan *c;
+
+	for (c = chan_map_first(&node->chans, &i);
+	     c;
+	     c = chan_map_next(&node->chans, &i)) {
+		if (is_chan_public(c))
 			return true;
+	}
 	return false;
 }
 
@@ -166,39 +175,33 @@ static bool node_has_public_channels(struct node *node)
  * we only send once we have a channel_update. */
 static bool node_has_broadcastable_channels(struct node *node)
 {
-	for (size_t i = 0; i < tal_count(node->chans); i++) {
-		if (!is_chan_public(node->chans[i]))
-			continue;
-		if (is_halfchan_defined(&node->chans[i]->half[0])
-		    || is_halfchan_defined(&node->chans[i]->half[1]))
-			return true;
-	}
-	return false;
-}
-
-static bool remove_channel_from_array(struct chan ***chans, const struct chan *c)
-{
-	size_t i, n;
+	struct chan_map_iter i;
+	struct chan *c;
 
-	n = tal_count(*chans);
-	for (i = 0; i < n; i++) {
-		if ((*chans)[i] != c)
+	for (c = chan_map_first(&node->chans, &i);
+	     c;
+	     c = chan_map_next(&node->chans, &i)) {
+		if (!is_chan_public(c))
 			continue;
-		n--;
-		memmove(*chans + i, *chans + i + 1, sizeof(**chans) * (n - i));
-		tal_resize(chans, n);
-		return true;
+		if (is_halfchan_defined(&c->half[0])
+		    || is_halfchan_defined(&c->half[1]))
+			return true;
 	}
 	return false;
 }
 
 static bool node_announce_predates_channels(const struct node *node)
 {
-	for (size_t i = 0; i < tal_count(node->chans); i++) {
-		if (!is_chan_announced(node->chans[i]))
+	struct chan_map_iter i;
+	struct chan *c;
+
+	for (c = chan_map_first(&node->chans, &i);
+	     c;
+	     c = chan_map_next(&node->chans, &i)) {
+		if (!is_chan_announced(c))
 			continue;
 
-		if (node->chans[i]->channel_announcement_index
+		if (c->channel_announcement_index
 		    < node->node_announcement_index)
 			return false;
 	}
@@ -216,11 +219,11 @@ static u64 persistent_broadcast(struct routing_state *rstate, const u8 *msg, u32
 static void remove_chan_from_node(struct routing_state *rstate,
 				  struct node *node, const struct chan *chan)
 {
-	if (!remove_channel_from_array(&node->chans, chan))
+	if (!chan_map_del(&node->chans, chan))
 		abort();
 
 	/* Last channel?  Simply delete node (and associated announce) */
-	if (tal_count(node->chans) == 0) {
+	if (node->chans.raw.elems == 0) {
 		tal_free(node);
 		return;
 	}
@@ -308,8 +311,8 @@ struct chan *new_chan(struct routing_state *rstate,
 	chan->sat = satoshis;
 	chan->local_disabled = false;
 
-	tal_arr_expand(&n2->chans, chan);
-	tal_arr_expand(&n1->chans, chan);
+	chan_map_add(&n2->chans, chan);
+	chan_map_add(&n1->chans, chan);
 
 	/* Populate with (inactive) connections */
 	init_half_chan(rstate, chan, n1idx);
@@ -520,15 +523,20 @@ find_route(const tal_t *ctx, struct routing_state *rstate,
 		for (n = node_map_first(rstate->nodes, &it);
 		     n;
 		     n = node_map_next(rstate->nodes, &it)) {
-			size_t num_edges = tal_count(n->chans);
-			for (i = 0; i < num_edges; i++) {
-				struct chan *chan = n->chans[i];
+			struct chan_map_iter i;
+			struct chan *chan;
+
+			for (chan = chan_map_first(&n->chans, &i);
+			     chan;
+			     chan = chan_map_next(&n->chans, &i)) {
 				int idx = half_chan_to(n, chan);
 
-				SUPERVERBOSE("Node %s edge %i/%zu",
+				SUPERVERBOSE("Node %s edge %s",
 					     type_to_string(tmpctx, struct pubkey,
 							    &n->id),
-					     i, num_edges);
+					     type_to_string(tmpctx,
+							    struct short_channel_id,
+							    &c->scid));
 
 				if (!hc_is_routable(chan, idx)) {
 					SUPERVERBOSE("...unroutable (local_disabled = %i, is_halfchan_enabled = %i, unroutable_until = %i",
@@ -1669,13 +1677,18 @@ void routing_failure(struct routing_state *rstate,
 				       type_to_string(tmpctx, struct pubkey,
 						      erring_node_pubkey));
 		} else {
+			struct chan_map_iter i;
+			struct chan *c;
+
 			status_trace("Deleting node %s",
 				     type_to_string(tmpctx,
 						    struct pubkey,
 						    &node->id));
-			for (size_t i = 0; i < tal_count(node->chans); ++i) {
+			for (c = chan_map_first(&node->chans, &i);
+			     c;
+			     c = chan_map_next(&node->chans, &i)) {
 				/* Set it up to be pruned. */
-				tal_steal(tmpctx, node->chans[i]);
+				tal_steal(tmpctx, c);
 			}
 		}
 	} else {
@@ -1745,9 +1758,18 @@ void route_prune(struct routing_state *rstate)
 void memleak_remove_routing_tables(struct htable *memtable,
 				   const struct routing_state *rstate)
 {
+	struct node *n;
+	struct node_map_iter nit;
+
 	memleak_remove_htable(memtable, &rstate->nodes->raw);
 	memleak_remove_htable(memtable, &rstate->pending_node_map->raw);
 	memleak_remove_uintmap(memtable, &rstate->broadcasts->broadcasts);
+
+	for (n = node_map_first(rstate->nodes, &nit);
+	     n;
+	     n = node_map_next(rstate->nodes, &nit)) {
+		memleak_remove_htable(memtable, &n->chans.raw);
+	}
 }
 #endif /* DEVELOPER */
 

gossipd/routing.h

@@ -85,6 +85,26 @@ static inline bool is_halfchan_enabled(const struct half_chan *hc)
 	return is_halfchan_defined(hc) && !(hc->channel_flags & ROUTING_FLAGS_DISABLED);
 }
 
+/* Container for per-node channel pointers.  Better cache performance
+* than uintmap, and we don't need ordering. */
+static inline const struct short_channel_id *chan_map_scid(const struct chan *c)
+{
+	return &c->scid;
+}
+
+static inline size_t hash_scid(const struct short_channel_id *scid)
+{
+	/* scids cost money to generate, so simple hash works here */
+	return (scid->u64 >> 32) ^ (scid->u64 >> 16) ^ scid->u64;
+}
+
+static inline bool chan_eq_scid(const struct chan *c,
+				const struct short_channel_id *scid)
+{
+	return short_channel_id_eq(scid, &c->scid);
+}
+HTABLE_DEFINE_TYPE(struct chan, chan_map_scid, hash_scid, chan_eq_scid, chan_map);
+
 struct node {
 	struct pubkey id;
 
@@ -95,7 +115,7 @@ struct node {
 	struct wireaddr *addresses;
 
 	/* Channels connecting us to other nodes */
-	struct chan **chans;
+	struct chan_map chans;
 
 	/* Temporary data for routefinding. */
 	struct {

gossipd/test/run-find_route.c

@@ -149,14 +149,16 @@ static struct chan *find_channel(struct routing_state *rstate UNUSED,
 					    const struct node *to,
 					    int *idx)
 {
-	int i, n;
+	struct chan_map_iter i;
+	struct chan *c;
 
 	*idx = pubkey_idx(&from->id, &to->id);
 
-	n = tal_count(to->chans);
-	for (i = 0; i < n; i++) {
-		if (to->chans[i]->nodes[*idx] == from)
-			return to->chans[i];
+	for (c = chan_map_first(&to->chans, &i);
+	     c;
+	     c = chan_map_next(&to->chans, &i)) {
+		if (c->nodes[*idx] == from)
+			return c;
 	}
 	return NULL;
 }