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 <equinox@opensourcerouting.org>

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)

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) $<

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,159 @@ 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);
+	graph_delete_node(node->cmdgraph, vector_slot(node->cmdgraph->nodes, 0));
+	vector_free(node->cmdgraph->nodes);
+	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():

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)
 

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

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

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 \