[ovs-dev] [PATCH ovn v5 2/2] tests: Add check-perf target
Numan Siddique
numans at ovn.org
Fri Jul 23 20:18:06 UTC 2021
On Wed, Jul 14, 2021 at 9:08 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> On 6/30/21 3:15 PM, Mark Gray wrote:
> > Add a suite of micro-benchmarks to aid a developer in understanding the
> > performance impact of any changes that they are making. They can be used to
> > help to understand the relative performance between two test runs on the same
> > test machine, but are not intended to give the absolute performance of OVN.
> >
> > To invoke the performance testsuite, run:
> >
> > $ make check-perf
> >
> > This will run all available performance tests.
> >
> > Additional metrics (e.g. memory, coverage, perf counters) may be added
> > in the future. Additional tests (e.g. additional topologies, ovn-controller
> > tests) may be added in the future.
> >
> > Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
> > ---
> >
> > Notes:
> > v2: create results directory to fix build error
> > v3: forgot to commit, create results directory to fix build error
> > v4: fix 0-day issues
> > remove `sudo` in Makefile
> > updated documentation
> >
> > Documentation/topics/testing.rst | 50 ++++++++
> > tests/.gitignore | 3 +
> > tests/automake.mk | 27 ++++
> > tests/perf-northd.at | 207 +++++++++++++++++++++++++++++++
> > tests/perf-testsuite.at | 26 ++++
> > 5 files changed, 313 insertions(+)
> > create mode 100644 tests/perf-northd.at
> > create mode 100644 tests/perf-testsuite.at
> >
> > diff --git a/Documentation/topics/testing.rst b/Documentation/topics/testing.rst
> > index be9e7c57331c..db265344a507 100644
> > --- a/Documentation/topics/testing.rst
> > +++ b/Documentation/topics/testing.rst
> > @@ -256,3 +256,53 @@ the following::
> > All the features documented under `Unit Tests`_ are available for the
> > datapath testsuites, except that the datapath testsuites do not
> > support running tests in parallel.
> > +
> > +Performance testing
> > +~~~~~~~~~~~~~~~~~~~
> > +
> > +OVN includes a suite of micro-benchmarks to aid a developer in understanding
> > +the performance impact of any changes that they are making. They can be used to
> > +help to understand the relative performance between two test runs on the same
> > +test machine, but are not intended to give the absolute performance of OVN.
> > +
> > +To invoke the performance testsuite, run::
> > +
> > + $ make check-perf
> > +
> > +This will run all available performance tests. Some of these tests may be
> > +long-running as they need to build complex logical network topologies. In order
> > +to speed up subsequent test runs, some objects (e.g. the Northbound DB) may be
> > +cached. In order to force the tests to rebuild all these objects, run::
> > +
> > + $ make check-perf TESTSUITEFLAGS="--rebuild"
> > +
> > +A typical workflow for a developer trying to improve the performance of OVN
> > +would be the following:
> > +
> > +0. Optional: Modify/add a performance test to buld the topology that you are
> > + benchmarking, if required.
> > +1. Run ``make check-perf TESTSUITEFLAGS="--rebuild"`` to generate cached
> > + databases (and complete a test run). The results of each test run are
> > + displayed on the screen at the end of the test run but are also saved in the
> > + file ``tests/perf-testsuite.dir/results``.
> > +
> > +.. note::
> > + This step may take some time depending on the number of tests that are being
> > + rebuilt, the complexity of the tests and the performance of the test
> > + machine. If you are only using one test, you can specify the test to run by
> > + adding the test number to the ``make`` command.
> > + (e.g. ``make check-perf TESTSUITEFLAGS="--rebuild <test number>"``)
> > +
> > +2. Run ``make check-perf`` to measure the performance metric that you are
> > + benchmarking against. If you are only using one test, you can specify the
> > + test to run by adding the test number to the ``make`` command.
> > + (e.g. ``make check-perf TESTSUITEFLAGS="--rebuild <test number>"``)
> > +3. Modify OVN code to implement the change that you believe will improve the
> > + performance.
> > +4. Go to Step 2. to continue making improvements.
> > +
> > +If, as a developer, you modify a performance test in a way that may change one
> > +of these cached objects, be sure to rebuild the test.
> > +
> > +The cached objects are stored under the relevant folder in
> > +``tests/perf-testsuite.dir/cached``.
> > diff --git a/tests/.gitignore b/tests/.gitignore
> > index 8479f9bb0f8f..65cb1c6e4fad 100644
> > --- a/tests/.gitignore
> > +++ b/tests/.gitignore
> > @@ -22,6 +22,9 @@
> > /system-offloads-testsuite
> > /system-offloads-testsuite.dir/
> > /system-offloads-testsuite.log
> > +/perf-testsuite
> > +/perf-testsuite.dir/
> > +/perf-testsuite.log
> > /test-aes128
> > /test-atomic
> > /test-bundle
> > diff --git a/tests/automake.mk b/tests/automake.mk
> > index a8ec64212791..5b890d644eeb 100644
> > --- a/tests/automake.mk
> > +++ b/tests/automake.mk
> > @@ -4,9 +4,11 @@ EXTRA_DIST += \
> > $(SYSTEM_TESTSUITE_AT) \
> > $(SYSTEM_KMOD_TESTSUITE_AT) \
> > $(SYSTEM_USERSPACE_TESTSUITE_AT) \
> > + $(PERF_TESTSUITE_AT) \
> > $(TESTSUITE) \
> > $(SYSTEM_KMOD_TESTSUITE) \
> > $(SYSTEM_USERSPACE_TESTSUITE) \
> > + $(PERF_TESTSUITE) \
> > tests/atlocal.in \
> > $(srcdir)/package.m4 \
> > $(srcdir)/tests/testsuite \
> > @@ -53,6 +55,10 @@ SYSTEM_TESTSUITE_AT = \
> > tests/system-ovn.at \
> > tests/system-ovn-kmod.at
> >
> > +PERF_TESTSUITE_AT = \
> > + tests/perf-testsuite.at \
> > + tests/perf-northd.at
> > +
> > check_SCRIPTS += tests/atlocal
> >
> > TESTSUITE = $(srcdir)/tests/testsuite
> > @@ -60,6 +66,9 @@ TESTSUITE_PATCH = $(srcdir)/tests/testsuite.patch
> > TESTSUITE_DIR = $(abs_top_builddir)/tests/testsuite.dir
> > SYSTEM_KMOD_TESTSUITE = $(srcdir)/tests/system-kmod-testsuite
> > SYSTEM_USERSPACE_TESTSUITE = $(srcdir)/tests/system-userspace-testsuite
> > +PERF_TESTSUITE = $(srcdir)/tests/perf-testsuite
> > +PERF_TESTSUITE_DIR = $(abs_top_builddir)/tests/perf-testsuite.dir
> > +PERF_TESTSUITE_RESULTS = $(PERF_TESTSUITE_DIR)/results
> > DISTCLEANFILES += tests/atconfig tests/atlocal
> >
> > AUTOTEST_PATH = $(ovs_builddir)/utilities:$(ovs_builddir)/vswitchd:$(ovs_builddir)/ovsdb:$(ovs_builddir)/vtep:tests:$(PTHREAD_WIN32_DIR_DLL):$(SSL_DIR):controller-vtep:northd:utilities:controller:ic
> > @@ -172,6 +181,20 @@ check-system-userspace: all
> >
> > clean-local:
> > test ! -f '$(TESTSUITE)' || $(SHELL) '$(TESTSUITE)' -C tests --clean
> > +
> > +check-perf: all
> > + @mkdir -p $(PERF_TESTSUITE_DIR)
> > + @echo > $(PERF_TESTSUITE_RESULTS)
> > + set $(SHELL) '$(PERF_TESTSUITE)' -C tests AUTOTEST_PATH='$(AUTOTEST_PATH)'; \
> > + "$$@" $(TESTSUITEFLAGS) -j1 || (test X'$(RECHECK)' = Xyes && "$$@" --recheck)
> > + @echo
> > + @echo '## -------------------- ##'
> > + @echo '## Performance Results ##'
> > + @echo '## -------------------- ##'
> > + @cat $(PERF_TESTSUITE_RESULTS)
> > + @echo
> > + @echo "Results can be found in $(PERF_TESTSUITE_RESULTS)"
> > +
> >
> > AUTOTEST = $(AUTOM4TE) --language=autotest
> >
> > @@ -194,6 +217,10 @@ $(SYSTEM_USERSPACE_TESTSUITE): package.m4 $(SYSTEM_TESTSUITE_AT) $(SYSTEM_USERSP
> > $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
> > $(AM_V_at)mv $@.tmp $@
> >
> > +$(PERF_TESTSUITE): package.m4 $(PERF_TESTSUITE_AT) $(COMMON_MACROS_AT)
> > + $(AM_V_GEN)$(AUTOTEST) -I '$(srcdir)' -o $@.tmp $@.at
> > + $(AM_V_at)mv $@.tmp $@
> > +
> > # The `:;' works around a Bash 3.2 bug when the output is not writeable.
> > $(srcdir)/package.m4: $(top_srcdir)/configure.ac
> > $(AM_V_GEN):;{ \
> > diff --git a/tests/perf-northd.at b/tests/perf-northd.at
> > new file mode 100644
> > index 000000000000..eb0c391c4650
> > --- /dev/null
> > +++ b/tests/perf-northd.at
> > @@ -0,0 +1,207 @@
> > +AT_BANNER([ovn-northd performance tests])
> > +
> > +# CACHE_NBDB()
> > +#
> > +# Save the current northbound database for future test runs.
> > +#
> > +m4_define([CACHE_NBDB],[
> > + mkdir -p ${at_suite_dir}/cached/${at_group}
> > + cp -f ${ovs_base}/ovn-nb/ovn-nb.db ${at_suite_dir}/cached/${at_group}/
> > +])
> > +
> > +# BUILD_NBDB([COMMANDS])
> > +#
> > +# Configure the northbound database using COMMANDS.
> > +#
> > +# BUILD_NBDB() will check if there is a cached nortbound database present
> > +# for this test group. If present, it will use the cached version. If the
> > +# testsuite was run using the '--rebuild' flag, it will force a rebuild of the
> > +# northbound database.
> > +#
> > +m4_define([BUILD_NBDB],[
> > + if [[ ! -f ${at_suite_dir}/cached/${at_group}/ovn-nb.db ]] || [[ $at_arg_rebuild != false ]]; then
> > + echo "Rebuild NBDB"
> > + $1
> > + CACHE_NBDB()
> > + else
> > + echo "Using cached NBDB"
> > + rm ${at_suite_dir}/cached/${at_group}/.ovn-nb.db.~lock~
> > + ovs-appctl -t ovn-nb/ovsdb-server ovsdb-server/remove-db OVN_Northbound
> > + ovs-appctl -t ovn-nb/ovsdb-server ovsdb-server/add-db ${at_suite_dir}/cached/${at_group}/ovn-nb.db
> > + fi
> > + ovn-nbctl --wait=sb sync
> > +])
> > +
> > +# PERF_RECORD_BANNER()
> > +#
> > +# Append standard banner to performance results.
> > +#
> > +m4_define([PERF_RECORD_START],[
> > + echo >> ${at_suite_dir}/results
> > + echo "$at_desc_line" >> ${at_suite_dir}/results
> > + echo " ---" >> ${at_suite_dir}/results
> > +])
> > +
> > +# PERF_RECORD_RESULT([KEY], [VALUE])
> > +#
> > +# Append KEY and VALUE to performance results.
> > +#
> > +m4_define([PERF_RECORD_RESULT],[
> > + echo " $1: $2" >> ${at_suite_dir}/results
> > +])
> > +
> > +m4_define([PARSE_STOPWATCH], [
> > + grep -A 6 $1 | grep $2 | sed 's/[[^0-9.]]*//g'
> > +])
> > +
> > +# PERF_RECORD_STOPWATCH([NAME], [METRIC])
> > +#
> > +# Append the value of the OVN stopwatch metric METRIC from stopwatch NAME
> > +# to performance results.
> > +#
> > +m4_define([PERF_RECORD_STOPWATCH], [
> > + PERF_RECORD_RESULT($3, [`ovn-appctl -t northd/NORTHD_TYPE stopwatch/show | PARSE_STOPWATCH($1, $2)`])
> > +])
> > +
> > +# PERF_RECORD()
> > +#
> > +# Append a number of metrics to performance results
> > +#
> > +m4_define([PERF_RECORD_STOP], [
> > + PERF_RECORD_STOPWATCH(ovnnb_db_run, ["Maximum"], [Maximum (NB)])
> > + PERF_RECORD_STOPWATCH(ovnnb_db_run, ["Short term average"], [Average (NB)])
>
> I think now that we have stopwatches for the whole northd loop (and for
> ovnsb_db_run) we might as well dump them here too. What do you think?
>
> Otherwise, the changes in this patch look good to me.
>
> Acked-by: Dumitru Ceara <dceara at redhat.com>
Thanks a lot Mark for this patch series and Dumitru for the reviews.
I think this will be helpful for developers.
I applied both the patches to the main branch with the below changes
to the 2nd patch. Please let me know
if something is wrong with the changes I've made. I can submit a
follow up patch if there are any mistakes.
I incorporated the changes suggested by Dumitru as well.
--------------------------------------------
diff --git a/tests/perf-northd.at b/tests/perf-northd.at
index eb0c391c46..74b69e9d4f 100644
--- a/tests/perf-northd.at
+++ b/tests/perf-northd.at
@@ -26,8 +26,8 @@ m4_define([BUILD_NBDB],[
else
echo "Using cached NBDB"
rm ${at_suite_dir}/cached/${at_group}/.ovn-nb.db.~lock~
- ovs-appctl -t ovn-nb/ovsdb-server ovsdb-server/remove-db OVN_Northbound
- ovs-appctl -t ovn-nb/ovsdb-server ovsdb-server/add-db
${at_suite_dir}/cached/${at_group}/ovn-nb.db
+ ovn-appctl -t ovn-nb/ovsdb-server ovsdb-server/remove-db OVN_Northbound
+ ovn-appctl -t ovn-nb/ovsdb-server ovsdb-server/add-db
${at_suite_dir}/cached/${at_group}/ovn-nb.db
fi
ovn-nbctl --wait=sb sync
])
@@ -51,7 +51,7 @@ m4_define([PERF_RECORD_RESULT],[
])
m4_define([PARSE_STOPWATCH], [
- grep -A 6 $1 | grep $2 | sed 's/[[^0-9.]]*//g'
+ grep $1 | sed 's/[[^0-9.]]*//g'
])
# PERF_RECORD_STOPWATCH([NAME], [METRIC])
@@ -60,7 +60,7 @@ m4_define([PARSE_STOPWATCH], [
# to performance results.
#
m4_define([PERF_RECORD_STOPWATCH], [
- PERF_RECORD_RESULT($3, [`ovn-appctl -t northd/NORTHD_TYPE
stopwatch/show | PARSE_STOPWATCH($1, $2)`])
+ PERF_RECORD_RESULT($3, [`ovn-appctl -t northd/NORTHD_TYPE
stopwatch/show $1 | PARSE_STOPWATCH($2)`])
])
# PERF_RECORD()
@@ -68,8 +68,12 @@ m4_define([PERF_RECORD_STOPWATCH], [
# Append a number of metrics to performance results
#
m4_define([PERF_RECORD_STOP], [
- PERF_RECORD_STOPWATCH(ovnnb_db_run, ["Maximum"], [Maximum (NB)])
- PERF_RECORD_STOPWATCH(ovnnb_db_run, ["Short term average"], [Average (NB)])
+ PERF_RECORD_STOPWATCH(ovnnb_db_run, ["Maximum"], [Maximum (NB in msec)])
+ PERF_RECORD_STOPWATCH(ovnnb_db_run, ["Short term average"],
[Average (NB in msec)])
+ PERF_RECORD_STOPWATCH(ovnsb_db_run, ["Maximum"], [Maximum (SB in msec)])
+ PERF_RECORD_STOPWATCH(ovnsb_db_run, ["Short term average"],
[Average (SB in msec)])
+ PERF_RECORD_STOPWATCH(ovn-northd-loop, ["Maximum"], [Maximum
(northd-loop in msec)])
+ PERF_RECORD_STOPWATCH(ovn-northd-loop, ["Short term average"],
[Average (northd-loop in msec)])
])
# OVN_NBCTL([NBCTL_COMMAND])
--------------------------------------------
Thanks
Numan
>
> Regards,
> Dumitru
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list