[ovs-dev] [PATCH ovn v5 2/2] tests: Add check-perf target
Mark Gray
mark.d.gray at redhat.com
Mon Jul 26 17:07:00 UTC 2021
On 23/07/2021 21:18, Numan Siddique wrote:
> 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'm not 100% sure of that but lets see. Either way, it can easily be
removed or let rot :)
Thanks for committing. TBH, I was planning to rework based on Dumitru's
comment about northd stopwatches but I see you made them
>
> 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.
Thanks, that all looks good.
>
> 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