libplugin-pay: use map for channel hints

This commit fixes a "FIXME: This is slow" in the pay plugin. For
nodes with many channels this is a tremendous improvement in pay
performance. PR #7611 improves payment performance from 15 seconds to
13.5 seconds on one of our nodes. This commit improves payment
performance from 13.5 seconds to 5.7 seconds.

Changelog-Fixed: Improved pathfinding speed for large nodes.

plugins/channel_hint.c

@@ -1,6 +1,34 @@
 #include "config.h"
+#include <common/memleak.h>
 #include <plugins/channel_hint.h>
 
+size_t channel_hint_hash(const struct short_channel_id_dir *out)
+{
+	struct siphash24_ctx ctx;
+	siphash24_init(&ctx, siphash_seed());
+	siphash24_update(&ctx, &out->scid.u64, sizeof(u64));
+	siphash24_update(&ctx, &out->dir, sizeof(int));
+	return siphash24_done(&ctx);
+}
+
+const struct short_channel_id_dir *channel_hint_keyof(const struct channel_hint *out)
+{
+	return &out->scid;
+}
+
+bool channel_hint_eq(const struct channel_hint *a,
+		     const struct short_channel_id_dir *b)
+{
+	return short_channel_id_eq(a->scid.scid, b->scid) &&
+		a->scid.dir == b->dir;
+}
+
+static void memleak_help_channel_hint_map(struct htable *memtable,
+					 struct channel_hint_map *channel_hints)
+{
+	memleak_scan_htable(memtable, &channel_hints->raw);
+}
+
 void channel_hint_to_json(const char *name, const struct channel_hint *hint,
 			  struct json_stream *dest)
 {
@@ -93,15 +121,10 @@ bool channel_hint_update(const struct timeabs now, struct channel_hint *hint)
 	       amount_msat_greater(capacity, hint->estimated_capacity);
 }
 
-struct channel_hint *channel_hint_set_find(struct channel_hint_set *self,
+struct channel_hint *channel_hint_set_find(const struct channel_hint_set *self,
 					   const struct short_channel_id_dir *scidd)
 {
-  for (size_t i=0; i<tal_count(self->hints); i++) {
-    struct channel_hint *hint = &self->hints[i];
-    if (short_channel_id_dir_eq(&hint->scid, scidd))
-      return hint;
-  }
-  return NULL;
+	return channel_hint_map_get(self->hints, scidd);
 }
 
 /* See header */
@@ -121,16 +144,16 @@ channel_hint_set_add(struct channel_hint_set *self, u32 timestamp,
 	old = channel_hint_set_find(self, scidd);
 	copy = tal_dup(tmpctx, struct channel_hint, old);
 	if (old == NULL) {
-		newhint = tal(tmpctx, struct channel_hint);
+		newhint = tal(self, struct channel_hint);
 		newhint->enabled = enabled;
 		newhint->scid = *scidd;
 		newhint->capacity = capacity;
 		if (estimated_capacity != NULL)
 			newhint->estimated_capacity = *estimated_capacity;
 		newhint->local = NULL;
 		newhint->timestamp = timestamp;
-		tal_arr_expand(&self->hints, *newhint);
-		return &self->hints[tal_count(self->hints) - 1];
+		channel_hint_map_add(self->hints, newhint);
+		return newhint;
 	} else if (old->timestamp <= timestamp) {
 		/* `local` is kept, since we do not pass in those
 		 * annotations here. */
@@ -184,18 +207,24 @@ struct channel_hint *channel_hint_from_json(const tal_t *ctx,
 struct channel_hint_set *channel_hint_set_new(const tal_t *ctx)
 {
 	struct channel_hint_set *set = tal(ctx, struct channel_hint_set);
-	set->hints = tal_arr(set, struct channel_hint, 0);
+	set->hints = tal(set, struct channel_hint_map);
+	channel_hint_map_init(set->hints);
+	memleak_add_helper(set->hints, memleak_help_channel_hint_map);
 	return set;
 }
 
 void channel_hint_set_update(struct channel_hint_set *set,
 			     const struct timeabs now)
 {
-	for (size_t i = 0; i < tal_count(set->hints); i++)
-		channel_hint_update(time_now(), &set->hints[i]);
+	struct channel_hint *hint;
+	struct channel_hint_map_iter iter;
+	for (hint = channel_hint_map_first(set->hints, &iter);
+	     hint;
+	     hint = channel_hint_map_next(set->hints, &iter))
+		channel_hint_update(now, hint);
 }
 
 size_t channel_hint_set_count(const struct channel_hint_set *set)
 {
-	return tal_count(set->hints);
+	return channel_hint_map_count(set->hints);
 }

plugins/channel_hint.h

@@ -45,11 +45,21 @@ struct channel_hint {
 	struct amount_sat capacity;
 };
 
+size_t channel_hint_hash(const struct short_channel_id_dir *out);
+
+const struct short_channel_id_dir *channel_hint_keyof(const struct channel_hint *out);
+
+bool channel_hint_eq(const struct channel_hint *a,
+		     const struct short_channel_id_dir *b);
+
+HTABLE_DEFINE_TYPE(struct channel_hint, channel_hint_keyof,
+	channel_hint_hash, channel_hint_eq, channel_hint_map)
+
 /* A collection of channel_hint instances, allowing us to handle and
  * update them more easily. */
 struct channel_hint_set {
-	/* tal_arr of channel_hints. */
-	struct channel_hint *hints;
+	/* htable of channel_hints, indexed by scid and direction. */
+	struct channel_hint_map *hints;
 };
 
 bool channel_hint_update(const struct timeabs now,
@@ -70,7 +80,7 @@ void channel_hint_set_update(struct channel_hint_set *set, const struct timeabs
 /**
  * Look up a `channel_hint` from a `channel_hint_set` for a scidd.
  */
-struct channel_hint *channel_hint_set_find(struct channel_hint_set *self,
+struct channel_hint *channel_hint_set_find(const struct channel_hint_set *self,
 					   const struct short_channel_id_dir *scidd);
 
 /**

plugins/libplugin-pay.c

@@ -614,11 +614,12 @@ payment_get_excluded_channels(const tal_t *ctx, struct payment *p)
 {
 	struct payment *root = payment_root(p);
 	struct channel_hint *hint;
+	struct channel_hint_map_iter iter;
 	struct short_channel_id_dir *res =
 	    tal_arr(ctx, struct short_channel_id_dir, 0);
-	for (size_t i = 0; i < tal_count(root->hints->hints); i++) {
-		hint = &root->hints->hints[i];
-
+	for (hint = channel_hint_map_first(root->hints->hints, &iter);
+	     hint;
+	     hint = channel_hint_map_next(root->hints->hints, &iter)) {
 		if (!hint->enabled)
 			tal_arr_expand(&res, hint->scid);
 
@@ -641,19 +642,6 @@ static const struct node_id *payment_get_excluded_nodes(const tal_t *ctx,
 	return root->excluded_nodes;
 }
 
-/* FIXME: This is slow! */
-static const struct channel_hint *find_hint(const struct channel_hint *hints,
-					    struct short_channel_id scid,
-					    int dir)
-{
-	for (size_t i = 0; i < tal_count(hints); i++) {
-		if (short_channel_id_eq(scid, hints[i].scid.scid)
-		    && dir == hints[i].scid.dir)
-			return &hints[i];
-	}
-	return NULL;
-}
-
 /* FIXME: This is slow! */
 static bool dst_is_excluded(const struct gossmap *gossmap,
 			    const struct gossmap_chan *c,
@@ -681,7 +669,7 @@ static bool payment_route_check(const struct gossmap *gossmap,
 				struct amount_msat amount,
 				struct payment *p)
 {
-	struct short_channel_id scid;
+	struct short_channel_id_dir scidd;
 	const struct channel_hint *hint;
 
 	if (dst_is_excluded(gossmap, c, dir, payment_root(p)->excluded_nodes))
@@ -690,8 +678,9 @@ static bool payment_route_check(const struct gossmap *gossmap,
 	if (dst_is_excluded(gossmap, c, dir, p->temp_exclusion))
 		return false;
 
-	scid = gossmap_chan_scid(gossmap, c);
-	hint = find_hint(payment_root(p)->hints->hints, scid, dir);
+	scidd.scid = gossmap_chan_scid(gossmap, c);
+	scidd.dir = dir;
+	hint = channel_hint_set_find(payment_root(p)->hints, &scidd);
 	if (!hint)
 		return true;
 
@@ -2618,10 +2607,7 @@ local_channel_hints_listpeerchannels(struct command *cmd, const char *buffer,
 	 * observations, and should re-enable some channels that would
 	 * otherwise start out as excluded and remain so until
 	 * forever. */
-
-	struct channel_hint *hints = payment_root(p)->hints->hints;
-	for (size_t i = 0; i < tal_count(hints); i++)
-		channel_hint_update(time_now(), &hints[i]);
+	channel_hint_set_update(payment_root(p)->hints, time_now());
 
 	payment_continue(p);
 	return command_still_pending(cmd);
@@ -2763,6 +2749,8 @@ static bool routehint_excluded(struct payment *p,
 	const struct short_channel_id_dir *chans =
 	    payment_get_excluded_channels(tmpctx, p);
 	const struct channel_hint_set *hints = payment_root(p)->hints;
+	struct short_channel_id_dir scidd;
+	struct channel_hint *hint;
 
 	/* Note that we ignore direction here: in theory, we could have
 	 * found that one direction of a channel is unavailable, but they
@@ -2802,15 +2790,12 @@ static bool routehint_excluded(struct payment *p,
 		 * know the exact capacity we need to send via this
 		 * channel, which is greater than the destination.
 		 */
-		for (size_t j = 0; j < tal_count(hints); j++) {
-			if (!short_channel_id_eq(hints->hints[j].scid.scid, r->short_channel_id))
-				continue;
-			/* We exclude on equality because we set the estimate
-			 * to the smallest failed attempt.  */
-			if (amount_msat_greater_eq(needed_capacity,
-						   hints->hints[j].estimated_capacity))
-				return true;
-		}
+		scidd.scid = r->short_channel_id;
+		scidd.dir = node_id_idx(&r->pubkey, p->pay_destination);
+		hint = channel_hint_set_find(hints, &scidd);
+		if (hint && amount_msat_greater_eq(needed_capacity,
+					   	   hint->estimated_capacity))
+			return true;
 	}
 	return false;
 }

plugins/test/run-route-calc.c

@@ -270,6 +270,12 @@ struct json_stream *jsonrpc_stream_fail(struct command *cmd UNNEEDED,
 /* Generated stub for jsonrpc_stream_success */
 struct json_stream *jsonrpc_stream_success(struct command *cmd UNNEEDED)
 { fprintf(stderr, "jsonrpc_stream_success called!\n"); abort(); }
+/* Generated stub for memleak_add_helper_ */
+void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
+						    const tal_t *)){ }
+/* Generated stub for memleak_scan_htable */
+void memleak_scan_htable(struct htable *memtable UNNEEDED, const struct htable *ht UNNEEDED)
+{ fprintf(stderr, "memleak_scan_htable called!\n"); abort(); }
 /* Generated stub for notleak_ */
 void *notleak_(void *ptr UNNEEDED, bool plus_children UNNEEDED)
 { fprintf(stderr, "notleak_ called!\n"); abort(); }

plugins/test/run-route-overlong.c

@@ -267,6 +267,12 @@ struct json_stream *jsonrpc_stream_fail(struct command *cmd UNNEEDED,
 /* Generated stub for jsonrpc_stream_success */
 struct json_stream *jsonrpc_stream_success(struct command *cmd UNNEEDED)
 { fprintf(stderr, "jsonrpc_stream_success called!\n"); abort(); }
+/* Generated stub for memleak_add_helper_ */
+void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
+						    const tal_t *)){ }
+/* Generated stub for memleak_scan_htable */
+void memleak_scan_htable(struct htable *memtable UNNEEDED, const struct htable *ht UNNEEDED)
+{ fprintf(stderr, "memleak_scan_htable called!\n"); abort(); }
 /* Generated stub for notleak_ */
 void *notleak_(void *ptr UNNEEDED, bool plus_children UNNEEDED)
 { fprintf(stderr, "notleak_ called!\n"); abort(); }