From de7db8a1bad04d66799b060091c9053b4054a082 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 8 May 2025 12:20:48 +0930 Subject: [PATCH] lightningd: keep a hash table for plugin notifications. This means we don't have to iterate through all plugins, making our "do we even have to construct this notification" optimization much more efficient. Signed-off-by: Rusty Russell --- lightningd/plugin.c | 86 ++++++++++++++++++++++++++++++++++----------- lightningd/plugin.h | 32 ++++++++++++++++- 2 files changed, 96 insertions(+), 22 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index f8f20773f..7623476bd 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -75,6 +75,8 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, p->plugin_idx = 0; p->dev_builtin_plugins_unimportant = false; p->want_db_transaction = true; + p->subscriptions = tal(p, struct plugin_subscription_htable); + plugin_subscription_htable_init(p->subscriptions); return p; } @@ -86,13 +88,13 @@ static void plugin_check_subscriptions(struct plugins *plugins, struct plugin *plugin) { for (size_t i = 0; i < tal_count(plugin->subscriptions); i++) { - const char *topic = plugin->subscriptions[i]; - if (!streq(topic, "*") - && !notifications_have_topic(plugins, topic)) + struct plugin_subscription *sub = &plugin->subscriptions[i]; + if (!streq(sub->topic, "*") + && !notifications_have_topic(plugins, sub->topic)) log_unusual( plugin->log, "topic '%s' is not a known notification topic", - topic); + sub->topic); } } @@ -273,6 +275,12 @@ static void destroy_plugin(struct plugin *p) /* Now free all the requests */ tal_free(reqs); + /* Remove any topics from the hash table */ + for (size_t i = 0; i < tal_count(p->subscriptions); i++) { + plugin_subscription_htable_del(p->plugins->subscriptions, + &p->subscriptions[i]); + } + /* If this was last one manifests were waiting for, handle deps */ if (p->plugin_state == AWAITING_GETMANIFEST_RESPONSE) check_plugins_manifests(p->plugins); @@ -1474,13 +1482,13 @@ static const char *plugin_subscriptions_add(struct plugin *plugin, plugin->subscriptions = NULL; return NULL; } - plugin->subscriptions = tal_arr(plugin, char *, 0); + plugin->subscriptions = tal_arr(plugin, struct plugin_subscription, 0); if (subscriptions->type != JSMN_ARRAY) { return tal_fmt(plugin, "\"result.subscriptions\" is not an array"); } json_for_each_arr(i, s, subscriptions) { - char *topic; + struct plugin_subscription sub; if (s->type != JSMN_STRING) { return tal_fmt(plugin, "result.subscriptions[%zu] is not a string: '%.*s'", i, @@ -1492,9 +1500,17 @@ static const char *plugin_subscriptions_add(struct plugin *plugin, * manifest, without checking that they exist, since * later plugins may also emit notifications of custom * types that we don't know about yet. */ - topic = json_strdup(plugin, plugin->buffer, s); - tal_arr_expand(&plugin->subscriptions, topic); + sub.topic = json_strdup(plugin, plugin->buffer, s); + sub.owner = plugin; + tal_arr_expand(&plugin->subscriptions, sub); } + + /* Now they won't move with reallocation, we can add to htable */ + for (i = 0; i < tal_count(plugin->subscriptions); i++) { + plugin_subscription_htable_add(plugin->plugins->subscriptions, + &plugin->subscriptions[i]); + } + return NULL; } @@ -2438,9 +2454,9 @@ static bool plugin_subscriptions_contains(struct plugin *plugin, const char *method) { for (size_t i = 0; i < tal_count(plugin->subscriptions); i++) { - if (streq(method, plugin->subscriptions[i]) + if (streq(method, plugin->subscriptions[i].topic) || is_asterix_notification(method, - plugin->subscriptions[i])) + plugin->subscriptions[i].topic)) return true; } @@ -2449,23 +2465,29 @@ static bool plugin_subscriptions_contains(struct plugin *plugin, bool plugins_anyone_cares(struct plugins *plugins, const char *method) { - struct plugin *p; + struct plugin_subscription_htable_iter it; if (!plugins) return false; - list_for_each(&plugins->plugins, p, list) { - if (plugin_subscriptions_contains(p, method)) - return true; - } - return false; + if (plugin_subscription_htable_getfirst(plugins->subscriptions, + method, &it) != NULL) + return true; + + /* Wildcards cover everything except "log" */ + if (streq(method, "log")) + return false; + return plugin_subscription_htable_getfirst(plugins->subscriptions, + "*", &it) != NULL; } bool plugin_single_notify(struct plugin *p, const struct jsonrpc_notification *n TAKES) { bool interested; - if (p->plugin_state == INIT_COMPLETE && plugin_subscriptions_contains(p, n->method)) { + + if (p->plugin_state == INIT_COMPLETE + && plugin_subscriptions_contains(p, n->method)) { plugin_send(p, json_stream_dup(p, n->stream, p->log)); interested = true; } else @@ -2480,15 +2502,37 @@ bool plugin_single_notify(struct plugin *p, void plugins_notify(struct plugins *plugins, const struct jsonrpc_notification *n TAKES) { - struct plugin *p; + struct plugin_subscription_htable_iter it; if (taken(n)) tal_steal(tmpctx, n); /* If we're shutting down, ld->plugins will be NULL */ - if (plugins) { - list_for_each(&plugins->plugins, p, list) { - plugin_single_notify(p, n); + if (!plugins) + return; + + for (struct plugin_subscription *sub + = plugin_subscription_htable_getfirst(plugins->subscriptions, + n->method, &it); + sub != NULL; + sub = plugin_subscription_htable_getnext(plugins->subscriptions, + n->method, &it)) { + if (sub->owner->plugin_state != INIT_COMPLETE) + continue; + plugin_send(sub->owner, json_stream_dup(sub->owner, n->stream, sub->owner->log)); + } + + /* "log" doesn't go to wildcards */ + if (!streq(n->method, "log")) { + for (struct plugin_subscription *sub + = plugin_subscription_htable_getfirst(plugins->subscriptions, + "*", &it); + sub != NULL; + sub = plugin_subscription_htable_getnext(plugins->subscriptions, + "*", &it)) { + if (sub->owner->plugin_state != INIT_COMPLETE) + continue; + plugin_send(sub->owner, json_stream_dup(sub->owner, n->stream, sub->owner->log)); } } } diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 15e75ae2e..14904e3f8 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -19,6 +19,33 @@ enum plugin_state { INIT_COMPLETE }; +struct plugin_subscription { + struct plugin *owner; + const char *topic; +}; + +static inline size_t hash_str(const char *str) +{ + return siphash24(siphash_seed(), str, strlen(str)); +} + +static inline const char *plugin_subscription_key(const struct plugin_subscription *ps) +{ + return ps->topic; +} + +static inline bool plugin_subscription_eq_topic(const struct plugin_subscription *ps, + const char *topic) +{ + return streq(ps->topic, topic); +} + +HTABLE_DEFINE_DUPS_TYPE(struct plugin_subscription, + plugin_subscription_key, + hash_str, + plugin_subscription_eq_topic, + plugin_subscription_htable); + /** * A plugin, exposed as a stub so we can pass it as an argument. */ @@ -71,7 +98,7 @@ struct plugin { const struct oneshot *timeout_timer; /* An array of subscribed topics */ - char **subscriptions; + struct plugin_subscription *subscriptions; /* Our pending requests by their request ID */ STRMAP(struct jsonrpc_request *) pending_requests; @@ -122,6 +149,9 @@ struct plugins { /* Index to show what order they were added in */ u64 plugin_idx; + /* Table of who subscribed to what hooks/notifications */ + struct plugin_subscription_htable *subscriptions; + /* Whether builtin plugins should be overridden as unimportant. */ bool dev_builtin_plugins_unimportant; };