-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
vtysh, lib: preprocess CLI graphs #16427
vtysh, lib: preprocess CLI graphs #16427
Conversation
Not used anywhere in FRR, kill it. Signed-off-by: David Lamparter <[email protected]>
Use alloced=0 to indicate that the array used in a vector is not in fact dynamically allocated memory (yet). Signed-off-by: David Lamparter <[email protected]>
I should make note of two things here:
|
Allow using "+1" when meaningful (i.e. cmd_graph_merge wants -1 or +1) Signed-off-by: David Lamparter <[email protected]>
The number of nodes in a graph will change as soon as cmd_graph_merge is supported as an operation, therefore size this dynamically. Signed-off-by: David Lamparter <[email protected]>
When merging graphs, it makes sense to allow starting with an empty one. Signed-off-by: David Lamparter <[email protected]>
Export cmd_graph_merge() to python code via graph1.merge(graph2). Signed-off-by: David Lamparter <[email protected]>
Make it a little easier to work on python code using this wrapper. Signed-off-by: David Lamparter <[email protected]>
Add len(graph) and graph[i] wrappers to access arbitrary nodes in a graph. Signed-off-by: David Lamparter <[email protected]>
There's a wrapper for nodes' outgoing pointers, but not incoming yet. Signed-off-by: David Lamparter <[email protected]>
FORK_TKN's join node is already exposed, mirror to expose JOIN_TKN's fork node. (contains minor cleanup to make checkpatch.pl shut up) Signed-off-by: David Lamparter <[email protected]>
Expose all of the struct members of cmd_token, and retrieve them dynamically rather than copying them around. The problem with copying them is that they can change as a result of merge(), and if there is an existing wrapper object around it will not have its copy updated to match. Signed-off-by: David Lamparter <[email protected]>
The command graph has its tail end nodes pointing at the `struct cmd_element` rather than a `struct cmd_token`. This is a bit weird to begin with, but becomes very annoying for the python bindings where there is just no `struct cmd_element`. Create a `CMD_ELEMENT_TKN` type for `cmd_token` instead, and replace the tail end token in the python bindings with an instance of that. Signed-off-by: David Lamparter <[email protected]>
There is entirely no point to these being conditional. And pull them up so the upcoming pre-parse code can work on a clean slate. Signed-off-by: David Lamparter <[email protected]>
9df460c
to
3306a7f
Compare
These are junk warnings and will need to be ignored sigh |
963da3d
to
890bff9
Compare
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 <[email protected]>
890bff9
to
d0de866
Compare
Looks like the remaining test failures are flaky timing. Faster |
ci:rerun |
I had this pushed in under #16501 closing this one |
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'
This improvement is particularly visible for users scripting
vtysh -c
calls, which notably includes topotests.