[ovs-dev] [PATCH ovn v5 2/2] tests: Add check-perf target

Dumitru Ceara dceara at redhat.com
Wed Jul 14 13:08:05 UTC 2021


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>

Regards,
Dumitru



More information about the dev mailing list