On Sun, Jan 18, 2026 at 4:02 PM Mark Michelson via dev < [email protected]> wrote:
> I recently made a logical flow refactor patch series and was asked to > benchmark it to ensure that we were not introducing any performance > regressions. In my case, I was lucky that the vital incremental engine > nodes already had stopwatches that I could use to compare performance. > > However, if I had wanted to test performance of other engine nodes, that > would not have been as easy. That's when I came up with this idea. With > this change set, we add stopwatches to every incremental engine node > automatically. With these stopwatches, we can measure recomputes and we > can measure change handlers. > > This will be a helpful metric to have when finding performance > regressions, and it can also offer insight into the time savings between > incremental processing and full recomputes. > > With the addition of new stopwatches to incremental nodes, it means that > there are now some redundant stopwatches. For instance, the "lflow_run" > stopwatch introduced in this commit now supersedes the existing > "build_lflows" stopwatch. However, I have not removed any existing > stopwatches in this series. The reason is that I do not want to break > any tests or other scripts that rely on these stopwatches. > > Mark Michelson (4): > inc-proc-eng: Add wrappers for callbacks. > inc-proc-eng: Add recompute stopwatch. > inc-proc-eng: Add change handler stopwatches. > inc-proc-eng: Add command to list node stopwatches. > > lib/inc-proc-eng.c | 60 ++++++++++++++++++++++++++++++++++++++++++---- > lib/inc-proc-eng.h | 5 +++- > 2 files changed, 60 insertions(+), 5 deletions(-) > > -- > 2.51.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Hi Mark, thank you for the series, maybe it's just me but I feel like this whole series could be a single commit. The changes are pretty small and it wouldn't be hard to review in a single piece. For the superseding of existing stopwatch, IMO we should remove the old ones even if it means updating a few tests. There is no need to stopwatch the same thing twice with the overhead it imposes. This also seem to apply only to northd: ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME DYNAMIC_ROUTES_RUN_STOPWATCH_NAME GROUP_ECMP_ROUTE_RUN_STOPWATCH_NAME LEARNED_ROUTE_SYNC_RUN_STOPWATCH_NAME BUILD_LFLOWS_STOPWATCH_NAME LR_NAT_RUN_STOPWATCH_NAME LR_STATEFUL_RUN_STOPWATCH_NAME LS_ARP_RUN_STOPWATCH_NAME LS_STATEFUL_RUN_STOPWATCH_NAME SYNC_METERS_RUN_STOPWATCH_NAME OVNNB_DB_RUN_STOPWATCH_NAME PORT_GROUP_RUN_STOPWATCH_NAME OVNSB_DB_RUN_STOPWATCH_NAME I don't think we guarantee this API to be stable, but I might be wrong about that. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
