From 0fc42415c24c12634b7e219ef80faf0223225c96 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 17 Apr 2019 17:05:33 +0930 Subject: [PATCH] gossipd/routing: remove BFG implementation. Now we can benchmark, and remove 500 bytes per node. MCP results from 5 runs, min-max(mean +/- stddev): store_load_msec:35093-37907(36146+/-1.1e+03) vsz_kb:555168 store_rewrite_sec:12.120000-13.750000(12.7+/-0.6) listnodes_sec:1.270000-1.370000(1.322+/-0.039) listchannels_sec:29.770000-31.600000(30.82+/-0.64) routing_sec:0.00 peer_write_all_sec:63.630000-67.850000(65.432+/-1.7) MCP notable changes from pre-Dijkstra (>1 stddev): -vsz_kb:577456 +vsz_kb:555168 -routing_sec:60.70 +routing_sec:12.04 Signed-off-by: Rusty Russell --- gossipd/routing.c | 234 +--------------------------- gossipd/routing.h | 11 -- gossipd/test/run-bench-find_route.c | 1 - 3 files changed, 7 insertions(+), 239 deletions(-) diff --git a/gossipd/routing.c b/gossipd/routing.c index a95632078..f7419fcb5 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -22,8 +22,6 @@ #define SUPERVERBOSE(...) #endif -bool only_dijkstra = false; - /* 365.25 * 24 * 60 / 10 */ #define BLOCKS_PER_YEAR 52596 @@ -439,19 +437,6 @@ struct unvisited { UINTMAP(struct node **) map; }; -static void clear_bfg(struct node_map *nodes) -{ - struct node *n; - struct node_map_iter it; - - for (n = node_map_first(nodes, &it); n; n = node_map_next(nodes, &it)) { - size_t i; - for (i = 0; i < ARRAY_SIZE(n->bfg); i++) { - n->bfg[i].total = INFINITE; - n->bfg[i].risk = AMOUNT_MSAT(0); - } - } -} /* Risk of passing through this channel. * @@ -604,49 +589,6 @@ static bool costs_less(struct amount_msat totala, return amount_msat_less(suma, sumb); } -/* We track totals, rather than costs. That's because the fee depends - * on the current amount passing through. */ -static void bfg_one_edge(struct node *node, - struct chan *chan, int idx, - double riskfactor, - double fuzz, const struct siphash_seed *base_seed, - size_t max_hops) -{ - size_t h; - const struct half_chan *c = &chan->half[idx]; - - for (h = 0; h < max_hops; h++) { - struct node *src; - struct amount_msat total, risk; - - if (!can_reach(c, &chan->scid, node->bfg[h].total, node->bfg[h].risk, - riskfactor, 1, fuzz, base_seed, &total, &risk)) - continue; - - /* nodes[0] is src for connections[0] */ - src = chan->nodes[idx]; - - if (costs_less(total, risk, NULL, - src->bfg[h + 1].total, - src->bfg[h + 1].risk, - NULL, - normal_cost_function)) { - SUPERVERBOSE("...%s can reach here hoplen %zu" - " total %s risk %s", - type_to_string(tmpctx, struct node_id, - &src->id), - h, - type_to_string(tmpctx, struct amount_msat, - &requiredcap), - type_to_string(tmpctx, struct amount_msat, - &risk)); - src->bfg[h+1].total = total; - src->bfg[h+1].risk = risk; - src->bfg[h+1].prev = chan; - } - } -} - /* Determine if the given half_chan is routable */ static bool hc_is_routable(struct routing_state *rstate, const struct chan *chan, int idx) @@ -1145,13 +1087,13 @@ out: /* riskfactor is already scaled to per-block amount */ static struct chan ** -find_route_dijkstra(const tal_t *ctx, struct routing_state *rstate, - const struct node_id *from, const struct node_id *to, - struct amount_msat msat, - double riskfactor, - double fuzz, const struct siphash_seed *base_seed, - size_t max_hops, - struct amount_msat *fee) +find_route(const tal_t *ctx, struct routing_state *rstate, + const struct node_id *from, const struct node_id *to, + struct amount_msat msat, + double riskfactor, + double fuzz, const struct siphash_seed *base_seed, + size_t max_hops, + struct amount_msat *fee) { struct node *src, *dst; struct unvisited *unvisited; @@ -1192,140 +1134,6 @@ find_route_dijkstra(const tal_t *ctx, struct routing_state *rstate, max_hops, fuzz, base_seed, route, fee); } -/* riskfactor is already scaled to per-block amount */ -static struct chan ** -find_route_bfg(const tal_t *ctx, struct routing_state *rstate, - const struct node_id *from, const struct node_id *to, - struct amount_msat msat, - double riskfactor, - double fuzz, const struct siphash_seed *base_seed, - size_t max_hops, - struct amount_msat *fee) -{ - struct chan **route; - struct node *n, *src, *dst; - struct node_map_iter it; - struct amount_msat best_total; - int runs, i, best; - - /* Note: we map backwards, since we know the amount of satoshi we want - * at the end, and need to derive how much we need to send. */ - dst = get_node(rstate, from); - src = get_node(rstate, to); - - if (!src) { - status_info("find_route: cannot find %s", - type_to_string(tmpctx, struct node_id, to)); - return NULL; - } else if (!dst) { - status_info("find_route: cannot find myself (%s)", - type_to_string(tmpctx, struct node_id, to)); - return NULL; - } else if (dst == src) { - status_info("find_route: this is %s, refusing to create empty route", - type_to_string(tmpctx, struct node_id, to)); - return NULL; - } - - if (max_hops > ROUTING_MAX_HOPS) { - status_info("find_route: max_hops huge amount %zu > %u", - max_hops, ROUTING_MAX_HOPS); - return NULL; - } - - /* Reset all the information. */ - clear_bfg(rstate->nodes); - - /* Bellman-Ford-Gibson: like Bellman-Ford, but keep values for - * every path length. */ - src->bfg[0].total = msat; - src->bfg[0].risk = AMOUNT_MSAT(0); - - for (runs = 0; runs < max_hops; runs++) { - SUPERVERBOSE("Run %i", runs); - /* Run through every edge. */ - for (n = node_map_first(rstate->nodes, &it); - n; - n = node_map_next(rstate->nodes, &it)) { - struct chan_map_iter i; - struct chan *chan; - - for (chan = first_chan(n, &i); - chan; - chan = next_chan(n, &i)) { - int idx = half_chan_to(n, chan); - - SUPERVERBOSE("Node %s edge %s", - type_to_string(tmpctx, struct node_id, - &n->id), - type_to_string(tmpctx, - struct short_channel_id, - &c->scid)); - - if (!hc_is_routable(rstate, chan, idx)) { - SUPERVERBOSE("...unroutable (local_disabled = %i, is_halfchan_enabled = %i, unroutable_until = %i", - is_chan_local_disabled(rstate, chan), - is_halfchan_enabled(&chan->half[idx]), - chan->half[idx].unroutable_until >= now); - continue; - } - bfg_one_edge(n, chan, idx, - riskfactor, fuzz, base_seed, - max_hops); - SUPERVERBOSE("...done"); - } - } - } - - best = 0; - best_total = INFINITE; - for (i = 0; i <= max_hops; i++) { - struct amount_msat total; - status_trace("%i hop solution: %s + %s", - i, - type_to_string(tmpctx, struct amount_msat, - &dst->bfg[i].total), - type_to_string(tmpctx, struct amount_msat, - &dst->bfg[i].risk)); - if (!amount_msat_add(&total, - dst->bfg[i].total, dst->bfg[i].risk)) - continue; - if (amount_msat_less(total, best_total)) { - best = i; - best_total = total; - } - } - status_trace("=> chose %i hop solution", best); - - /* No route? */ - if (amount_msat_greater_eq(best_total, INFINITE)) { - status_trace("find_route: No route to %s", - type_to_string(tmpctx, struct node_id, to)); - return NULL; - } - - /* We (dst) don't charge ourselves fees, so skip first hop */ - n = other_node(dst, dst->bfg[best].prev); - if (!amount_msat_sub(fee, n->bfg[best-1].total, msat)) { - status_broken("Could not subtract %s - %s for fee", - type_to_string(tmpctx, struct amount_msat, - &n->bfg[best-1].total), - type_to_string(tmpctx, struct amount_msat, &msat)); - return NULL; - } - - /* Lay out route */ - route = tal_arr(ctx, struct chan *, best); - for (i = 0, n = dst; - i < best; - n = other_node(n, n->bfg[best-i].prev), i++) { - route[i] = n->bfg[best-i].prev; - } - assert(n == src); - - return route; -} - /* Checks that key is valid, and signed this hash */ static bool check_signed_hash_nodeid(const struct sha256_double *hash, const secp256k1_ecdsa_signature *signature, @@ -2396,34 +2204,6 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann) return NULL; } -static struct chan ** -find_route(const tal_t *ctx, struct routing_state *rstate, - const struct node_id *from, const struct node_id *to, - struct amount_msat msat, - double riskfactor, - double fuzz, const struct siphash_seed *base_seed, - size_t max_hops, - struct amount_msat *fee) -{ - struct chan **rd, **rbfg; - - rd = find_route_dijkstra(ctx, rstate, from, to, msat, - riskfactor, - fuzz, base_seed, max_hops, fee); - if (only_dijkstra) - return rd; - - /* Make sure they match */ - rbfg = find_route_bfg(ctx, rstate, from, to, msat, - riskfactor, - fuzz, base_seed, max_hops, fee); - /* FIXME: Dijkstra can give overlength! */ - if (tal_count(rd) < max_hops) - assert(memeq(rd, tal_bytelen(rd), rbfg, tal_bytelen(rbfg))); - tal_free(rd); - return rbfg; -} - struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate, const struct node_id *source, const struct node_id *destination, diff --git a/gossipd/routing.h b/gossipd/routing.h index 83572bbfb..ce021da87 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -110,15 +110,6 @@ struct node { } chans; /* Temporary data for routefinding. */ - struct { - /* Total to get to here from target. */ - struct amount_msat total; - /* Total risk premium of this route. */ - struct amount_msat risk; - /* Where that came from. */ - struct chan *prev; - } bfg[ROUTING_MAX_HOPS+1]; - struct { /* Total to get to here from target. */ struct amount_msat total; @@ -436,6 +427,4 @@ static inline void local_enable_chan(struct routing_state *rstate, /* Helper to convert on-wire addresses format to wireaddrs array */ struct wireaddr *read_addresses(const tal_t *ctx, const u8 *ser); -/* Temporary to set routing algo */ -extern bool only_dijkstra; #endif /* LIGHTNING_GOSSIPD_ROUTING_H */ diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index 5ee918548..de025d06c 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/gossipd/test/run-bench-find_route.c @@ -222,7 +222,6 @@ int main(int argc, char *argv[]) "Run perfme-start and perfme-stop around benchmark"); opt_parse(&argc, argv, opt_log_stderr_exit); - only_dijkstra = true; if (argc > 1) num_nodes = atoi(argv[1]);