[ovs-dev] [PATCH ovn v3 2/3] ovn-northd: Add useful stopwatches

Dumitru Ceara dceara at redhat.com
Thu Jun 24 15:33:25 UTC 2021


On 6/18/21 10:52 AM, Mark Gray wrote:
> For performance measurement, it is useful to understand the
> length of time required to complete a number of key code paths
> in ovn-northd.c. Add stopwatches to measure these timings.
> 
> Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
> ---

Acked-by: Dumitru Ceara <dceara at redhat.com>

I only have one real nit on this patch (below).  Except for that here
are some more random thoughts for potential follow ups.

I think we might benefit from a even more granular measurement, e.g., in
some of the tests I was doing a while ago build_datapaths() was also
taking up a significant amount of time.

Some more interesting ones to measure are in
build_lswitch_and_lrouter_flows(), the loops that call
build_lswitch_*_by_od/op().  I don't know however how we could deal with
the parallel case though.

Thanks,
Dumitru

>  northd/ovn-northd-ddlog.c | 15 +++++++++++++++
>  northd/ovn-northd.c       | 20 ++++++++++++++++++++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/northd/ovn-northd-ddlog.c b/northd/ovn-northd-ddlog.c
> index a4f2960bdcb8..7c552d516550 100644
> --- a/northd/ovn-northd-ddlog.c
> +++ b/northd/ovn-northd-ddlog.c
> @@ -37,6 +37,7 @@
>  #include "ovsdb-parser.h"
>  #include "ovsdb-types.h"
>  #include "simap.h"
> +#include "stopwatch.h"
>  #include "stream-ssl.h"
>  #include "stream.h"
>  #include "unixctl.h"
> @@ -50,6 +51,10 @@ VLOG_DEFINE_THIS_MODULE(ovn_northd);
>  #include "northd/ovn-northd-ddlog-nb.inc"
>  #include "northd/ovn-northd-ddlog-sb.inc"
>  
> +#define NORTHD_LOOP_STOPWATCH_NAME "ovn-northd-loop"
> +#define OVNNB_DB_RUN_STOPWATCH_NAME "ovnnb_db_run"
> +#define OVNSB_DB_RUN_STOPWATCH_NAME "ovnsb_db_run"

A bit of a nit: would it make sense to not duplicate these in both
northd versions and just add them to a (potentially new) common header file?



More information about the dev mailing list