From 890bff987c08c231f887a096c0ec265aabaaf60c Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 20 Jul 2024 18:30:06 -0700 Subject: [PATCH] vtysh, lib: preprocess CLI graphs Store a parsed and built graph of the CLI nodes in vtysh, rather than parsing and building that graph every time vtysh starts up. This provides a 3x to 5x reduction in vtysh startup overhead: `vtysh -c 'configure' -c 'interface lo' -c 'do show version'` - before: 92.9M cycles, 1114 samples - after: 16.5M cycles, 330 samples This improvement is particularly visible for users scripting `vtysh -c` calls, which notably includes topotests. Signed-off-by: David Lamparter --- lib/command.h | 11 +-- lib/subdir.am | 25 ++++++- python/xref2vtysh.py | 164 ++++++++++++++++++++++++++++++++++++++++--- python/xrelfo.py | 18 +++-- tests/lib/subdir.am | 2 +- vtysh/.gitignore | 1 + vtysh/subdir.am | 3 - 7 files changed, 200 insertions(+), 24 deletions(-) diff --git a/lib/command.h b/lib/command.h index f364f1e8fa2a..cb105c656c18 100644 --- a/lib/command.h +++ b/lib/command.h @@ -256,7 +256,7 @@ struct cmd_node { /* helper defines for end-user DEFUN* macros */ #define DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attrs, dnum) \ - static const struct cmd_element cmdname = { \ + const struct cmd_element cmdname = { \ .string = cmdstr, \ .func = funcname, \ .doc = helpstr, \ @@ -283,7 +283,7 @@ struct cmd_node { /* DEFPY variants */ #define DEFPY_ATTR(funcname, cmdname, cmdstr, helpstr, attr) \ - DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attr, 0) \ + static DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attr, 0) \ funcdecl_##funcname #define DEFPY(funcname, cmdname, cmdstr, helpstr) \ @@ -310,7 +310,7 @@ struct cmd_node { #define DEFUN_ATTR(funcname, cmdname, cmdstr, helpstr, attr) \ DEFUN_CMD_FUNC_DECL(funcname) \ - DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attr, 0) \ + static DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attr, 0) \ DEFUN_CMD_FUNC_TEXT(funcname) #define DEFUN(funcname, cmdname, cmdstr, helpstr) \ @@ -347,7 +347,8 @@ struct cmd_node { /* DEFUN + DEFSH */ #define DEFUNSH_ATTR(daemon, funcname, cmdname, cmdstr, helpstr, attr) \ DEFUN_CMD_FUNC_DECL(funcname) \ - DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attr, daemon) \ + static DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attr, \ + daemon) \ DEFUN_CMD_FUNC_TEXT(funcname) #define DEFUNSH(daemon, funcname, cmdname, cmdstr, helpstr) \ @@ -359,7 +360,7 @@ struct cmd_node { /* ALIAS macro which define existing command's alias. */ #define ALIAS_ATTR(funcname, cmdname, cmdstr, helpstr, attr) \ - DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attr, 0) + static DEFUN_CMD_ELEMENT(funcname, cmdname, cmdstr, helpstr, attr, 0) #define ALIAS(funcname, cmdname, cmdstr, helpstr) \ ALIAS_ATTR(funcname, cmdname, cmdstr, helpstr, 0) diff --git a/lib/subdir.am b/lib/subdir.am index 3264f31af7a4..4bcce9a2b05a 100644 --- a/lib/subdir.am +++ b/lib/subdir.am @@ -504,20 +504,41 @@ SUFFIXES += .xref %.xref: % $(CLIPPY) $(AM_V_XRELFO) $(CLIPPY) $(top_srcdir)/python/xrelfo.py $(WERROR) $(XRELFO_FLAGS) -o $@ $< +# these run up to a total of 350k lines at the time of writing. That's a lot +# for the compiler to chug down - enough to warrant splitting it up so it can +# benefit from parallel build. +vtysh_cmd_split = \ + vtysh/vtysh_cmd.1.c \ + vtysh/vtysh_cmd.2.c \ + vtysh/vtysh_cmd.3.c \ + vtysh/vtysh_cmd.4.c \ + vtysh/vtysh_cmd.5.c \ + vtysh/vtysh_cmd.6.c \ + vtysh/vtysh_cmd.7.c \ + vtysh/vtysh_cmd.8.c \ + # end + # dependencies added in python/makefile.py frr.xref: - $(AM_V_XRELFO) $(CLIPPY) $(top_srcdir)/python/xrelfo.py -o $@ -c vtysh/vtysh_cmd.c $^ + $(AM_V_XRELFO) $(CLIPPY) $(top_srcdir)/python/xrelfo.py -o $@ $^ \ + -c vtysh/vtysh_cmd.c $(vtysh_cmd_split) all-am: frr.xref clean-xref: -rm -rf $(xrefs) frr.xref clean-local: clean-xref -CLEANFILES += vtysh/vtysh_cmd.c vtysh/vtysh_cmd.c: frr.xref @test -f $@ || rm -f frr.xref || true @test -f $@ || $(MAKE) $(AM_MAKEFLAGS) frr.xref +vtysh/vtysh_cmd.%.c: vtysh/vtysh_cmd.c + @test -f $@ || rm -f vtysh/vtysh_cmd.c || true + @test -f $@ || $(MAKE) $(AM_MAKEFLAGS) vtysh/vtysh_cmd.c + +nodist_vtysh_vtysh_SOURCES = vtysh/vtysh_cmd.c $(vtysh_cmd_split) +CLEANFILES += vtysh/vtysh_cmd.c $(vtysh_cmd_split) + ## automake's "ylwrap" is a great piece of GNU software... not. .l.c: $(AM_V_LEX)$(am__skiplex) $(LEXCOMPILE) $< diff --git a/python/xref2vtysh.py b/python/xref2vtysh.py index 75d9ccf367df..e4828ee23877 100644 --- a/python/xref2vtysh.py +++ b/python/xref2vtysh.py @@ -26,6 +26,8 @@ except ImportError: pass +import _clippy + frr_top_src = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) # vtysh needs to know which daemon(s) to send commands to. For lib/, this is @@ -58,6 +60,16 @@ #include "linklist.h" #include "vtysh/vtysh.h" + +#pragma GCC visibility push(internal) + +#define MAKE_VECTOR(name, len, ...) \\ + static void * name ## _vitems[] = { __VA_ARGS__ }; \\ + static struct _vector name = { \\ + .active = len, \\ + .count = len, \\ + .index = name ## _vitems, \\ + } """ if sys.stderr.isatty(): @@ -335,23 +347,157 @@ def output_defs(cls, ofd): ofd.write(entry.get_def()) @classmethod - def output_install(cls, ofd, nodes): - ofd.write("\nvoid vtysh_init_cmd(void)\n{\n") + def output_node_graph(cls, ofd, node, cmds, splitfile): + graph = _clippy.Graph(None) + + for _, cmd in sorted(cmds.items()): + cg = _clippy.Graph(cmd.cmd, cmd._spec["doc"], cmd.name) + graph.merge(cg) + + if len(graph) <= 2: + return [] + + ofd.write("\n") + ofd.write(f"static struct cmd_token ctkn_{node}[];\n") + ofd.write(f"static struct graph_node gn_{node}[];\n") + ofd.write("\n") + + vectors = [] + cmdels = set() + + ofd.write(f"static struct cmd_token ctkn_{node}[] = {'{'}\n") + for i, token in enumerate(graph): + vectors.append( + ( + list(i.idx for i in token.next()), + list(i.idx for i in token.prev()), + ) + ) - for name, items in sorted(nodes.items_named()): - for item in sorted(items.values(), key=lambda i: i.name): - ofd.write("\tinstall_element(%s, &%s_vtysh);\n" % (name, item.name)) + if token.type == "CMD_ELEMENT_TKN": + ofd.write(f"\t{'{'} /* [{i}] = {token.text} */ {'}'},\n") + cmdels.add(token.text) + continue + + ofd.write(f"\t{'{'} /* [{i}] */\n\t\t.type = {token.type},\n") + if token.attr: + ofd.write(f"\t\t.attr = {token.attr},\n") + if token.allowrepeat: + ofd.write(f"\t\t.allowrepeat = true,\n") + if token.varname_src: + ofd.write(f"\t\t.varname_src = {token.varname_src},\n") + if token.text: + ofd.write(f'\t\t.text = (char *)"{c_escape(token.text)}",\n') + if token.desc: + ofd.write(f'\t\t.desc = (char *)"{c_escape(token.desc)}",\n') + if token.min: + ofd.write(f"\t\t.min = {token.min},\n") + if token.max: + ofd.write(f"\t\t.max = {token.max},\n") + if token.varname: + ofd.write(f'\t\t.varname = (char *)"{c_escape(token.varname)}",\n') + + if token.type == "FORK_TKN": + fj = token.join() + ofd.write(f"\t\t.forkjoin = &gn_{node}[{fj.idx}],\n") + if token.type == "JOIN_TKN": + fj = token.fork() + ofd.write(f"\t\t.forkjoin = &gn_{node}[{fj.idx}],\n") + + ofd.write(f"\t{'}'},\n") + + ofd.write("};\n\n") + + if splitfile: + for cmdel in sorted(cmdels): + ofd.write(f"extern struct cmd_element {cmdel}_vtysh;\n") + ofd.write("\n") + + for i, next_prev in enumerate(vectors): + n, p = next_prev + items = ", ".join(f"&gn_{node}[{i}]" for i in n) + ofd.write(f"MAKE_VECTOR(gn_{node}_{i}_next, {len(n)}, {items});\n") + items = ", ".join(f"&gn_{node}[{i}]" for i in p) + ofd.write(f"MAKE_VECTOR(gn_{node}_{i}_prev, {len(p)}, {items});\n") + + ofd.write(f"\nstatic struct graph_node gn_{node}[] = {'{'}\n") + for i, token in enumerate(graph): + ofd.write("\t{\n") + ofd.write(f"\t\t.from = &gn_{node}_{i}_prev,\n") + ofd.write(f"\t\t.to = &gn_{node}_{i}_next,\n") + if token.type == "CMD_ELEMENT_TKN": + ofd.write(f"\t\t.data = (void *)&{token.text}_vtysh,\n") + else: + ofd.write(f"\t\t.data = &ctkn_{node}[{i}],\n") + ofd.write("\t},\n") + ofd.write("};\n") + + items = ", ".join(f"&gn_{node}[{i}]" for i in range(0, len(graph))) + ofd.write(f"MAKE_VECTOR(gvec_{node}, {len(graph)}, {items});\n") + + ofd.write( + f""" +{"extern " if splitfile else "static "}void install_{node}(void);\n +{"" if splitfile else "static "}void install_{node}(void)\n +{'{'} + unsigned node_id = {node}; + struct cmd_node *node; + + assert(node_id < vector_active(cmdvec)); + node = vector_slot(cmdvec, node_id); + assert(node); + assert(vector_active(node->cmdgraph->nodes) == 1); + node->cmdgraph->nodes = &gvec_{node}; +{'}'} +""" + ) - ofd.write("}\n") + return [node] @classmethod - def run(cls, xref, ofd): - ofd.write(vtysh_cmd_head) + def run(cls, xref, ofds): + for ofd in ofds: + ofd.write(vtysh_cmd_head) + + ofd = ofds.pop(0) NodeDict.load_nodenames() nodes = cls.load(xref) cls.output_defs(ofd) - cls.output_install(ofd, nodes) + + out_nodes = [] + for nodeid, cmds in nodes.items(): + node = nodes.nodename(nodeid) + + if ofds: + gfd, splitfile = ofds[nodeid % len(ofds)], True + else: + gfd, splitfile = ofd, False + + # install_element(VIEW_NODE, x) implies install_element(ENABLE_NODE, x) + # this needs to be handled here. + if node == "ENABLE_NODE": + nodeid_view = list( + k for k, v in nodes.nodenames.items() if v == "VIEW_NODE" + ) + assert len(nodeid_view) == 1 + cmds.update(nodes[nodeid_view[0]]) + + out_nodes.extend(cls.output_node_graph(gfd, node, cmds, splitfile)) + + out_nodes.sort() + + if ofds: + ofd.write("\n") + for name in out_nodes: + ofd.write(f"extern void install_{name}(void);\n") + + ofd.write("\nvoid vtysh_init_cmd(void)\n{\n") + + for name in out_nodes: + ofd.write(f"\tinstall_{name}();\n") + + ofd.write("}\n") def main(): diff --git a/python/xrelfo.py b/python/xrelfo.py index 07cd74071f8e..5f7616f25093 100644 --- a/python/xrelfo.py +++ b/python/xrelfo.py @@ -447,7 +447,9 @@ def main(): argp = argparse.ArgumentParser(description="FRR xref ELF extractor") argp.add_argument("-o", dest="output", type=str, help="write JSON output") argp.add_argument("--out-by-file", type=str, help="write by-file JSON output") - argp.add_argument("-c", dest="vtysh_cmds", type=str, help="write vtysh_cmd.c") + argp.add_argument( + "-c", dest="vtysh_cmds", type=str, help="write vtysh_cmd.c", nargs="*" + ) argp.add_argument("-Wlog-format", action="store_const", const=True) argp.add_argument("-Wlog-args", action="store_const", const=True) argp.add_argument("-Werror", action="store_const", const=True) @@ -528,9 +530,17 @@ def _main(args): os.rename(args.out_by_file + ".tmp", args.out_by_file) if args.vtysh_cmds: - with open(args.vtysh_cmds + ".tmp", "w") as fd: - CommandEntry.run(out, fd) - os.rename(args.vtysh_cmds + ".tmp", args.vtysh_cmds) + fds = [] + for filename in args.vtysh_cmds: + fds.append(open(filename + ".tmp", "w")) + + CommandEntry.run(out, fds) + + while fds: + fds.pop(0).close() + for filename in args.vtysh_cmds: + os.rename(filename + ".tmp", filename) + if args.Werror and CommandEntry.warn_counter: sys.exit(1) diff --git a/tests/lib/subdir.am b/tests/lib/subdir.am index 185b89507932..1a21684f16ca 100644 --- a/tests/lib/subdir.am +++ b/tests/lib/subdir.am @@ -100,7 +100,7 @@ check_PROGRAMS += tests/lib/cli/test_commands tests_lib_cli_test_commands_CFLAGS = $(TESTS_CFLAGS) tests_lib_cli_test_commands_CPPFLAGS = $(TESTS_CPPFLAGS) tests_lib_cli_test_commands_LDADD = $(ALL_TESTS_LDADD) -nodist_tests_lib_cli_test_commands_SOURCES = tests/lib/cli/test_commands_defun.c +nodist_tests_lib_cli_test_commands_SOURCES = tests/lib/cli/test_commands_defun.c $(vtysh_cmd_split) tests_lib_cli_test_commands_SOURCES = tests/lib/cli/test_commands.c tests/helpers/c/prng.c tests/lib/cli/test_commands_defun.c: vtysh/vtysh_cmd.c @$(MKDIR_P) tests/lib/cli diff --git a/vtysh/.gitignore b/vtysh/.gitignore index a6c3d4abc6ca..9cbd248f2f72 100644 --- a/vtysh/.gitignore +++ b/vtysh/.gitignore @@ -1,5 +1,6 @@ vtysh vtysh_cmd.c +vtysh_cmd.*.c # does not exist anymore - remove 2023-10-04 or so extract.pl diff --git a/vtysh/subdir.am b/vtysh/subdir.am index 2eae16d6291c..d39987eb83c8 100644 --- a/vtysh/subdir.am +++ b/vtysh/subdir.am @@ -17,9 +17,6 @@ vtysh_vtysh_SOURCES = \ vtysh/vtysh_user.c \ vtysh/vtysh_config.c \ # end -nodist_vtysh_vtysh_SOURCES = \ - vtysh/vtysh_cmd.c \ - # end noinst_HEADERS += \ vtysh/vtysh.h \