Skip to content

Commit 46c551e

Browse files
committed
lightningd: optimize find_cmd.
We have a reasonable number of commands now, and we *already* keep a strmap for the usage strings. So simply keep the usage and the command in the map, and skip the array. tests/test_coinmoves.py::test_generate_coinmoves (2,000,000, sqlite3): Time (from start to end of l2 node): 95 seconds (was 102) Worst latency: 4.5 seconds tests/test_coinmoves.py::test_generate_coinmoves (2,000,000, Postgres): Time (from start to end of l2 node): 231 seconds Worst latency: 4.8 seconds Note the values compare against 25.09.2 (Postgres): sqlite3: Time (from start to end of l2 node): 403 seconds Postgres: Time (from start to end of l2 node): 671 seconds Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1 parent 54cab8e commit 46c551e

1 file changed

Lines changed: 88 additions & 91 deletions

File tree

lightningd/jsonrpc.c

Lines changed: 88 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,13 @@ struct json_connection {
100100
struct list_head jsouts;
101101
};
102102

103+
/* We don't put usage inside struct json_command as it's good practice
104+
* to have those const. */
105+
struct cmd_and_usage {
106+
const struct json_command *command;
107+
const char *usage;
108+
};
109+
103110
/**
104111
* `jsonrpc` encapsulates the entire state of the JSON-RPC interface,
105112
* including a list of methods that the interface supports (can be
@@ -108,11 +115,9 @@ struct json_connection {
108115
*/
109116
struct jsonrpc {
110117
struct io_listener *rpc_listener;
111-
struct json_command **commands;
112118

113-
/* Map from json command names to usage strings: we don't put this inside
114-
* struct json_command as it's good practice to have those const. */
115-
STRMAP(const char *) usagemap;
119+
/* Can't be const: we set ->usage later */
120+
STRMAP(struct cmd_and_usage *) cmdmap;
116121
};
117122

118123
/* The command itself usually owns the stream, because jcon may get closed.
@@ -400,85 +405,74 @@ static const struct json_command dev_command = {
400405
};
401406
AUTODATA(json_command, &dev_command);
402407

403-
static size_t num_cmdlist;
404-
405-
static struct json_command **get_cmdlist(void)
408+
static struct json_command **get_cmdlist(size_t *num_cmdlist)
406409
{
407410
static struct json_command **cmdlist;
408411
if (!cmdlist)
409-
cmdlist = autodata_get(json_command, &num_cmdlist);
412+
cmdlist = autodata_get(json_command, num_cmdlist);
410413

411414
return cmdlist;
412415
}
413416

414-
static void json_add_help_command(struct command *cmd,
415-
struct json_stream *response,
416-
struct json_command *json_command)
417+
struct json_help_info {
418+
struct command *cmd;
419+
struct json_stream *response;
420+
};
421+
422+
/* Used as a strmap_iterate function: returns true to continue */
423+
static bool json_add_help_command(const char *cmdname,
424+
struct cmd_and_usage *cmd,
425+
struct json_help_info *hinfo)
417426
{
418427
char *usage;
419428

420429
/* If they disallow deprecated APIs, don't even list them */
421-
if (!command_deprecated_out_ok(cmd, NULL,
422-
json_command->depr_start,
423-
json_command->depr_end)) {
424-
return;
425-
}
426-
427-
usage = tal_fmt(cmd, "%s%s %s",
428-
json_command->name,
429-
json_command->depr_start ? " (DEPRECATED!)" : "",
430-
strmap_get(&cmd->ld->jsonrpc->usagemap,
431-
json_command->name));
432-
json_object_start(response, NULL);
433-
json_add_string(response, "command", usage);
434-
json_object_end(response);
435-
}
436-
437-
static const struct json_command *find_command(struct json_command **commands,
438-
const char *cmdname)
439-
{
440-
for (size_t i = 0; i < tal_count(commands); i++) {
441-
if (streq(cmdname, commands[i]->name))
442-
return commands[i];
430+
if (!command_deprecated_out_ok(hinfo->cmd, NULL,
431+
cmd->command->depr_start,
432+
cmd->command->depr_end)) {
433+
return true;
443434
}
444-
return NULL;
445-
}
446435

447-
static int compare_commands_name(struct json_command *const *a,
448-
struct json_command *const *b, void *unused)
449-
{
450-
return strcmp((*a)->name, (*b)->name);
436+
assert(cmd->command);
437+
assert(!streq(cmd->command->name, "xxxxX"));
438+
assert(!streq(cmd->usage, "xxxxX"));
439+
usage = tal_fmt(tmpctx, "%s%s %s",
440+
cmd->command->name,
441+
cmd->command->depr_start ? " (DEPRECATED!)" : "",
442+
cmd->usage);
443+
json_object_start(hinfo->response, NULL);
444+
json_add_string(hinfo->response, "command", usage);
445+
json_object_end(hinfo->response);
446+
return true;
451447
}
452448

453449
static struct command_result *json_help(struct command *cmd,
454450
const char *buffer,
455451
const jsmntok_t *obj UNNEEDED,
456452
const jsmntok_t *params)
457453
{
458-
struct json_stream *response;
459454
const char *cmdname;
460-
struct json_command **commands;
461-
const struct json_command *one_cmd;
455+
struct cmd_and_usage *one_cmd;
456+
struct json_help_info hinfo;
462457

463458
if (!param_check(cmd, buffer, params,
464459
p_opt("command", param_string, &cmdname),
465460
NULL))
466461
return command_param_failed();
467462

468-
commands = cmd->ld->jsonrpc->commands;
469463
if (cmdname) {
470-
one_cmd = find_command(commands, cmdname);
464+
one_cmd = strmap_get(&cmd->ld->jsonrpc->cmdmap, cmdname);
471465
if (!one_cmd)
472466
return command_fail(cmd, JSONRPC2_METHOD_NOT_FOUND,
473467
"Unknown command %s",
474468
cmdname);
475469
if (!command_deprecated_in_ok(cmd, NULL,
476-
one_cmd->depr_start,
477-
one_cmd->depr_end))
470+
one_cmd->command->depr_start,
471+
one_cmd->command->depr_end))
478472
return command_fail(cmd, JSONRPC2_METHOD_NOT_FOUND,
479473
"Deprecated command %s",
480474
cmdname);
481-
if (!cmd->ld->developer && one_cmd->dev_only)
475+
if (!cmd->ld->developer && one_cmd->command->dev_only)
482476
return command_fail(cmd, JSONRPC2_METHOD_NOT_FOUND,
483477
"Developer-only command %s",
484478
cmdname);
@@ -488,31 +482,32 @@ static struct command_result *json_help(struct command *cmd,
488482
if (command_check_only(cmd))
489483
return command_check_done(cmd);
490484

491-
asort(commands, tal_count(commands), compare_commands_name, NULL);
492-
493-
response = json_stream_success(cmd);
494-
json_array_start(response, "help");
495-
for (size_t i = 0; i < tal_count(commands); i++) {
496-
if (!one_cmd || one_cmd == commands[i])
497-
json_add_help_command(cmd, response, commands[i]);
485+
hinfo.cmd = cmd;
486+
hinfo.response = json_stream_success(cmd);
487+
json_array_start(hinfo.response, "help");
488+
if (one_cmd)
489+
json_add_help_command(cmdname, one_cmd, &hinfo);
490+
else {
491+
strmap_iterate(&cmd->ld->jsonrpc->cmdmap,
492+
json_add_help_command, &hinfo);
498493
}
499-
json_array_end(response);
494+
json_array_end(hinfo.response);
500495

501496
/* Tell cli this is simple enough to be formatted flat for humans */
502-
json_add_string(response, "format-hint", "simple");
497+
json_add_string(hinfo.response, "format-hint", "simple");
503498

504-
return command_success(cmd, response);
499+
return command_success(cmd, hinfo.response);
505500
}
506501

507502
static const struct json_command *find_cmd(const struct jsonrpc *rpc,
508503
const char *buffer,
509504
const jsmntok_t *tok)
510505
{
511-
struct json_command **commands = rpc->commands;
506+
const struct cmd_and_usage *cmd;
512507

513-
for (size_t i = 0; i < tal_count(commands); i++)
514-
if (json_tok_streq(buffer, tok, commands[i]->name))
515-
return commands[i];
508+
cmd = strmap_getn(&rpc->cmdmap, buffer + tok->start, tok->end - tok->start);
509+
if (cmd)
510+
return cmd->command;
516511
return NULL;
517512
}
518513

@@ -1286,27 +1281,26 @@ static struct io_plan *incoming_jcon_connected(struct io_conn *conn,
12861281

12871282
static void destroy_json_command(struct json_command *command, struct jsonrpc *rpc)
12881283
{
1289-
strmap_del(&rpc->usagemap, command->name, NULL);
1290-
for (size_t i = 0; i < tal_count(rpc->commands); i++) {
1291-
if (rpc->commands[i] == command) {
1292-
tal_arr_remove(&rpc->commands, i);
1293-
return;
1294-
}
1295-
}
1296-
abort();
1284+
struct cmd_and_usage *cmd;
1285+
1286+
if (!strmap_del(&rpc->cmdmap, command->name, &cmd))
1287+
abort();
1288+
tal_free(cmd);
12971289
}
12981290

1299-
static bool command_add(struct jsonrpc *rpc, struct json_command *command)
1291+
static struct cmd_and_usage *command_add(struct jsonrpc *rpc, struct json_command *command)
13001292
{
1301-
size_t count = tal_count(rpc->commands);
1293+
struct cmd_and_usage *cmd;
13021294

13031295
/* Check that we don't clobber a method */
1304-
for (size_t i = 0; i < count; i++)
1305-
if (streq(rpc->commands[i]->name, command->name))
1306-
return false;
1296+
if (strmap_get(&rpc->cmdmap, command->name))
1297+
return NULL;
13071298

1308-
tal_arr_expand(&rpc->commands, command);
1309-
return true;
1299+
cmd = tal(rpc, struct cmd_and_usage);
1300+
cmd->command = command;
1301+
cmd->usage = NULL;
1302+
strmap_add(&rpc->cmdmap, command->name, cmd);
1303+
return cmd;
13101304
}
13111305

13121306
/* Built-in commands get called to construct usage string via param() */
@@ -1323,22 +1317,23 @@ static void setup_command_usage(struct lightningd *ld,
13231317
dummy->json_cmd = command;
13241318
res = command->dispatch(dummy, NULL, NULL, NULL);
13251319
assert(res == &param_failed);
1326-
assert(strmap_get(&ld->jsonrpc->usagemap, command->name));
1320+
assert(strmap_get(&ld->jsonrpc->cmdmap, command->name)->usage);
13271321
}
13281322

13291323
bool jsonrpc_command_add(struct jsonrpc *rpc, struct json_command *command,
13301324
const char *usage TAKES)
13311325
{
1332-
const char *unescaped;
1326+
struct cmd_and_usage *cmd;
13331327

1334-
if (!command_add(rpc, command))
1328+
cmd = command_add(rpc, command);
1329+
if (!cmd)
13351330
return false;
13361331

1337-
unescaped = json_escape_unescape_len(command, usage, strlen(usage));
1338-
if (!unescaped)
1332+
cmd->usage = json_escape_unescape_len(cmd, usage, strlen(usage));
1333+
if (!cmd->usage) {
1334+
tal_free(cmd);
13391335
return false;
1340-
1341-
strmap_add(&rpc->usagemap, command->name, unescaped);
1336+
}
13421337
tal_add_destructor2(command, destroy_json_command, rpc);
13431338
return true;
13441339
}
@@ -1355,22 +1350,22 @@ static bool jsonrpc_command_add_perm(struct lightningd *ld,
13551350

13561351
static void destroy_jsonrpc(struct jsonrpc *jsonrpc)
13571352
{
1358-
strmap_clear(&jsonrpc->usagemap);
1353+
strmap_clear(&jsonrpc->cmdmap);
13591354
}
13601355

13611356
static void memleak_help_jsonrpc(struct htable *memtable,
13621357
struct jsonrpc *jsonrpc)
13631358
{
1364-
memleak_scan_strmap(memtable, &jsonrpc->usagemap);
1359+
memleak_scan_strmap(memtable, &jsonrpc->cmdmap);
13651360
}
13661361

13671362
void jsonrpc_setup(struct lightningd *ld)
13681363
{
1369-
struct json_command **commands = get_cmdlist();
1364+
size_t num_cmdlist;
1365+
struct json_command **commands = get_cmdlist(&num_cmdlist);
13701366

13711367
ld->jsonrpc = tal(ld, struct jsonrpc);
1372-
strmap_init(&ld->jsonrpc->usagemap);
1373-
ld->jsonrpc->commands = tal_arr(ld->jsonrpc, struct json_command *, 0);
1368+
strmap_init(&ld->jsonrpc->cmdmap);
13741369
for (size_t i=0; i<num_cmdlist; i++) {
13751370
if (!jsonrpc_command_add_perm(ld, ld->jsonrpc, commands[i]))
13761371
fatal("Cannot add duplicate command %s",
@@ -1407,9 +1402,11 @@ void command_log(struct command *cmd, enum log_level level,
14071402

14081403
void command_set_usage(struct command *cmd, const char *usage TAKES)
14091404
{
1410-
usage = tal_strdup(cmd->ld, usage);
1411-
if (!strmap_add(&cmd->ld->jsonrpc->usagemap, cmd->json_cmd->name, usage))
1412-
fatal("Two usages for command %s?", cmd->json_cmd->name);
1405+
struct cmd_and_usage *cmd_and_usage;
1406+
1407+
cmd_and_usage = strmap_get(&cmd->ld->jsonrpc->cmdmap, cmd->json_cmd->name);
1408+
assert(!cmd_and_usage->usage);
1409+
cmd_and_usage->usage = tal_strdup(cmd_and_usage, usage);
14131410
}
14141411

14151412
bool command_check_only(const struct command *cmd)

0 commit comments

Comments
 (0)