Browse Source

gossipd: use explicit destructor for struct chan.

Each destructor2 costs 40 bytes, and struct chan is only 120 bytes.  So
this drops our memory usage quite a bit:

MCP bench results change:
   -vsz_kb:580004-580016(580006+/-4.8)
   +vsz_kb:533148

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
htlc_accepted_hook
Rusty Russell 6 years ago
parent
commit
d1f43d993a
  1. 2
      gossipd/gossip_store.c
  2. 4
      gossipd/gossipd.c
  3. 33
      gossipd/routing.c
  4. 3
      gossipd/routing.h
  5. 10
      gossipd/test/run-bench-find_route.c
  6. 7
      gossipd/test/run-find_route-specific.c
  7. 7
      gossipd/test/run-find_route.c
  8. 7
      gossipd/test/run-overlong.c

2
gossipd/gossip_store.c

@ -604,7 +604,7 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs)
bad = "Bad channel_delete scid"; bad = "Bad channel_delete scid";
goto truncate; goto truncate;
} }
tal_free(c); free_chan(rstate, c);
stats[3]++; stats[3]++;
break; break;
case WIRE_GOSSIPD_LOCAL_ADD_CHANNEL: case WIRE_GOSSIPD_LOCAL_ADD_CHANNEL:

4
gossipd/gossipd.c

@ -2806,9 +2806,9 @@ static struct io_plan *handle_outpoint_spent(struct io_conn *conn,
"spent", "spent",
type_to_string(msg, struct short_channel_id, &scid)); type_to_string(msg, struct short_channel_id, &scid));
/* Freeing is sufficient since everything else is allocated off /* Freeing is sufficient since everything else is allocated off
* of the channel and the destructor takes care of unregistering * of the channel and this takes care of unregistering
* the channel */ * the channel */
tal_free(chan); free_chan(rstate, chan);
/* We put a tombstone marker in the channel store, so we don't /* We put a tombstone marker in the channel store, so we don't
* have to replay blockchain spends on restart. */ * have to replay blockchain spends on restart. */
gossip_store_add_channel_delete(rstate->broadcasts->gs, &scid); gossip_store_add_channel_delete(rstate->broadcasts->gs, &scid);

33
gossipd/routing.c

@ -220,7 +220,7 @@ static void destroy_node(struct node *node, struct routing_state *rstate)
/* These remove themselves from chans[]. */ /* These remove themselves from chans[]. */
while ((c = first_chan(node, &i)) != NULL) while ((c = first_chan(node, &i)) != NULL)
tal_free(c); free_chan(rstate, c);
/* Free htable if we need. */ /* Free htable if we need. */
if (node_uses_chan_map(node)) if (node_uses_chan_map(node))
@ -344,7 +344,9 @@ static void remove_chan_from_node(struct routing_state *rstate,
} }
} }
static void destroy_chan(struct chan *chan, struct routing_state *rstate) /* We used to make this a tal_add_destructor2, but that costs 40 bytes per
* chan, and we only ever explicitly free it anyway. */
void free_chan(struct routing_state *rstate, struct chan *chan)
{ {
remove_chan_from_node(rstate, chan->nodes[0], chan); remove_chan_from_node(rstate, chan->nodes[0], chan);
remove_chan_from_node(rstate, chan->nodes[1], chan); remove_chan_from_node(rstate, chan->nodes[1], chan);
@ -358,6 +360,7 @@ static void destroy_chan(struct chan *chan, struct routing_state *rstate)
/* Remove from local_disabled_map if it's there. */ /* Remove from local_disabled_map if it's there. */
chan_map_del(&rstate->local_disabled_map, chan); chan_map_del(&rstate->local_disabled_map, chan);
tal_free(chan);
} }
static void init_half_chan(struct routing_state *rstate, static void init_half_chan(struct routing_state *rstate,
@ -416,8 +419,6 @@ struct chan *new_chan(struct routing_state *rstate,
init_half_chan(rstate, chan, !n1idx); init_half_chan(rstate, chan, !n1idx);
uintmap_add(&rstate->chanmap, scid->u64, chan); uintmap_add(&rstate->chanmap, scid->u64, chan);
tal_add_destructor2(chan, destroy_chan, rstate);
return chan; return chan;
} }
@ -1395,7 +1396,8 @@ bool routing_add_channel_announcement(struct routing_state *rstate,
} }
/* Pretend it didn't exist, for the moment. */ /* Pretend it didn't exist, for the moment. */
tal_free(chan); if (chan)
free_chan(rstate, chan);
uc = tal(rstate, struct unupdated_channel); uc = tal(rstate, struct unupdated_channel);
uc->channel_announce = tal_dup_arr(uc, u8, msg, tal_count(msg), 0); uc->channel_announce = tal_dup_arr(uc, u8, msg, tal_count(msg), 0);
@ -2286,6 +2288,8 @@ void routing_failure(struct routing_state *rstate,
enum onion_type failcode, enum onion_type failcode,
const u8 *channel_update) const u8 *channel_update)
{ {
struct chan **pruned = tal_arr(tmpctx, struct chan *, 0);
status_trace("Received routing failure 0x%04x (%s), " status_trace("Received routing failure 0x%04x (%s), "
"erring node %s, " "erring node %s, "
"channel %s/%u", "channel %s/%u",
@ -2331,7 +2335,7 @@ void routing_failure(struct routing_state *rstate,
&node->id)); &node->id));
for (c = first_chan(node, &i); c; c = next_chan(node, &i)) { for (c = first_chan(node, &i); c; c = next_chan(node, &i)) {
/* Set it up to be pruned. */ /* Set it up to be pruned. */
tal_steal(tmpctx, c); tal_arr_expand(&pruned, c);
} }
} }
} else { } else {
@ -2356,9 +2360,13 @@ void routing_failure(struct routing_state *rstate,
struct short_channel_id, struct short_channel_id,
scid)); scid));
/* Set it up to be deleted. */ /* Set it up to be deleted. */
tal_steal(tmpctx, chan); tal_arr_expand(&pruned, chan);
} }
} }
/* Now free all the chans and maybe even nodes. */
for (size_t i = 0; i < tal_count(pruned); i++)
free_chan(rstate, pruned[i]);
} }
@ -2367,7 +2375,7 @@ void route_prune(struct routing_state *rstate)
u64 now = gossip_time_now(rstate).ts.tv_sec; u64 now = gossip_time_now(rstate).ts.tv_sec;
/* Anything below this highwater mark ought to be pruned */ /* Anything below this highwater mark ought to be pruned */
const s64 highwater = now - rstate->prune_timeout; const s64 highwater = now - rstate->prune_timeout;
const tal_t *pruned = tal(NULL, char); struct chan **pruned = tal_arr(tmpctx, struct chan *, 0);
struct chan *chan; struct chan *chan;
struct unupdated_channel *uc; struct unupdated_channel *uc;
u64 idx; u64 idx;
@ -2394,7 +2402,7 @@ void route_prune(struct routing_state *rstate)
: now - chan->half[1].bcast.timestamp); : now - chan->half[1].bcast.timestamp);
/* This may perturb iteration so do outside loop. */ /* This may perturb iteration so do outside loop. */
tal_steal(pruned, chan); tal_arr_expand(&pruned, chan);
} }
} }
@ -2403,12 +2411,13 @@ void route_prune(struct routing_state *rstate)
uc; uc;
uc = uintmap_after(&rstate->unupdated_chanmap, &idx)) { uc = uintmap_after(&rstate->unupdated_chanmap, &idx)) {
if (uc->added.ts.tv_sec < highwater) { if (uc->added.ts.tv_sec < highwater) {
tal_steal(pruned, uc); tal_arr_expand(&pruned, chan);
} }
} }
/* This frees all the chans and maybe even nodes. */ /* Now free all the chans and maybe even nodes. */
tal_free(pruned); for (size_t i = 0; i < tal_count(pruned); i++)
free_chan(rstate, pruned[i]);
} }
#if DEVELOPER #if DEVELOPER

3
gossipd/routing.h

@ -54,6 +54,9 @@ struct chan {
struct amount_sat sat; struct amount_sat sat;
}; };
/* Use this instead of tal_free(chan)! */
void free_chan(struct routing_state *rstate, struct chan *chan);
/* A local channel can exist which isn't announced; normal channels are only /* A local channel can exist which isn't announced; normal channels are only
* created once we have both an announcement *and* an update. */ * created once we have both an announcement *and* an update. */
static inline bool is_chan_public(const struct chan *chan) static inline bool is_chan_public(const struct chan *chan)

10
gossipd/test/run-bench-find_route.c

@ -131,9 +131,10 @@ static void add_connection(struct routing_state *rstate,
memcpy((char *)&scid + (!idx) * sizeof(to), &to, sizeof(to)); memcpy((char *)&scid + (!idx) * sizeof(to), &to, sizeof(to));
chan = get_channel(rstate, &scid); chan = get_channel(rstate, &scid);
if (!chan) if (!chan) {
chan = new_chan(rstate, &scid, &nodes[from], &nodes[to], chan = new_chan(rstate, &scid, &nodes[from], &nodes[to],
AMOUNT_SAT(1000000)); AMOUNT_SAT(1000000));
}
c = &chan->half[idx]; c = &chan->half[idx];
c->base_fee = base_fee; c->base_fee = base_fee;
@ -277,6 +278,13 @@ int main(int argc, char *argv[])
if (route_lengths[i]) if (route_lengths[i])
printf(" Length %zu: %zu\n", i, route_lengths[i]); printf(" Length %zu: %zu\n", i, route_lengths[i]);
/* Since we omitted destructors on these, clean up manually */
u64 idx;
for (struct chan *chan = uintmap_first(&rstate->chanmap, &idx);
chan;
chan = uintmap_after(&rstate->chanmap, &idx))
free_chan(rstate, chan);
tal_free(tmpctx); tal_free(tmpctx);
secp256k1_context_destroy(secp256k1_ctx); secp256k1_context_destroy(secp256k1_ctx);
opt_free_table(); opt_free_table();

7
gossipd/test/run-find_route-specific.c

@ -261,6 +261,13 @@ int main(void)
ROUTING_MAX_HOPS, &fee); ROUTING_MAX_HOPS, &fee);
assert(!route); assert(!route);
/* Since we omitted destructors on these, clean up manually */
u64 idx;
for (struct chan *chan = uintmap_first(&rstate->chanmap, &idx);
chan;
chan = uintmap_after(&rstate->chanmap, &idx))
free_chan(rstate, chan);
tal_free(tmpctx); tal_free(tmpctx);
secp256k1_context_destroy(secp256k1_ctx); secp256k1_context_destroy(secp256k1_ctx);
return 0; return 0;

7
gossipd/test/run-find_route.c

@ -278,6 +278,13 @@ int main(void)
assert(channel_is_between(route[1], &d, &c)); assert(channel_is_between(route[1], &d, &c));
assert(amount_msat_eq(fee, AMOUNT_MSAT(0 + 6))); assert(amount_msat_eq(fee, AMOUNT_MSAT(0 + 6)));
/* Since we omitted destructors on these, clean up manually */
u64 idx;
for (struct chan *chan = uintmap_first(&rstate->chanmap, &idx);
chan;
chan = uintmap_after(&rstate->chanmap, &idx))
free_chan(rstate, chan);
tal_free(tmpctx); tal_free(tmpctx);
secp256k1_context_destroy(secp256k1_ctx); secp256k1_context_destroy(secp256k1_ctx);
return 0; return 0;

7
gossipd/test/run-overlong.c

@ -197,6 +197,13 @@ int main(void)
last_fee = fee; last_fee = fee;
} }
/* Since we omitted destructors on these, clean up manually */
u64 idx;
for (struct chan *chan = uintmap_first(&rstate->chanmap, &idx);
chan;
chan = uintmap_after(&rstate->chanmap, &idx))
free_chan(rstate, chan);
tal_free(tmpctx); tal_free(tmpctx);
secp256k1_context_destroy(secp256k1_ctx); secp256k1_context_destroy(secp256k1_ctx);
return 0; return 0;

Loading…
Cancel
Save