Re: [PATCH 2/4] Perf test framework

Doug Evans
2013-10-25T06:32:24Z

Hi. Just a few nits inline.
I need to download the patch and play with it!

On Wed, Oct 16, 2013 at 12:09 AM, Yao Qi <yao@codesourcery.com> wrote:
> This patch adds a basic framework to do performance testing for GDB.
> perftest.py is about the test case, testresult.py is about test
> results, and how are they saved. reporter.py is about how results
> are reported (in what format). measure.py is about measuring the
> execution of tests by a collection of measurements.
>
> In V3, there are some changes,
>
> - Add wall time measurement, cpu time measurement and vmsize
> measurement.
> - Rename SingleStatisticTestCase to TestCaseWithBasicMeasurements,
> which measures cpu time, wall time, and memory (vmsize).
> - GDB_PERFORMANCE=run|compile|both to control the mode of perf
> testing.
> - New GDB_PERFORMANCE_TIMEOUT to specify the timeout.
> - Split proc prepare to proc compile and startup.
> - Disable GC while doing measurements.
>
> In V2, there are several changes to address Doug and Sanimir's
> comments.
>
> - Add copyright header and docstring in perftest/__init__.py
> - Remove config.py.
> - Fix docstring format.
> - Rename classes "SingleVariable" to "SingleStatistic".
> - Don't extend gdb.Function in class TestCase. Add a new method run
> to run the test case so that we can pass parameters to test.
> - Allow to customize whether to warm up and to append test log.
> - Move time measurement into test harness. Add a new class
> Measurement for a specific measurement and a new class Measure to
> measure them for a given test case.
> - A new class ResultFactory to create instances of TestResult.
> - New file lib/perftest.exp, which is to do some preparations and
> cleanups to simplify each *.exp file.
> - Skip compilation step if GDB_PERFORMANCE_SKIP_COMPILE is set.
>
> gdb/testsuite/
>
> * lib/perftest.exp: New.
> * gdb.perf/lib/perftest/__init__.py: New.
> * gdb.perf/lib/perftest/measure.py: New.
> * gdb.perf/lib/perftest/perftest.py: New.
> * gdb.perf/lib/perftest/reporter.py: New.
> * gdb.perf/lib/perftest/testresult.py: New.
> ---
> gdb/testsuite/gdb.perf/lib/perftest/__init__.py | 17 +++
> gdb/testsuite/gdb.perf/lib/perftest/measure.py | 146 ++++++++++++++++++++
> gdb/testsuite/gdb.perf/lib/perftest/perftest.py | 72 ++++++++++
> gdb/testsuite/gdb.perf/lib/perftest/reporter.py | 64 +++++++++
> gdb/testsuite/gdb.perf/lib/perftest/testresult.py | 57 ++++++++
> gdb/testsuite/lib/perftest.exp | 148 +++++++++++++++++++++
> 6 files changed, 504 insertions(+), 0 deletions(-)
> create mode 100644 gdb/testsuite/gdb.perf/lib/perftest/__init__.py
> create mode 100644 gdb/testsuite/gdb.perf/lib/perftest/measure.py
> create mode 100644 gdb/testsuite/gdb.perf/lib/perftest/perftest.py
> create mode 100644 gdb/testsuite/gdb.perf/lib/perftest/reporter.py
> create mode 100644 gdb/testsuite/gdb.perf/lib/perftest/testresult.py
> create mode 100644 gdb/testsuite/lib/perftest.exp
>
> diff --git a/gdb/testsuite/gdb.perf/lib/perftest/__init__.py b/gdb/testsuite/gdb.perf/lib/perftest/__init__.py
> new file mode 100644
> index 0000000..7739a3e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/lib/perftest/__init__.py
> @@ -0,0 +1,17 @@
> +# Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +"""
> +GDB performance testing framework.
> +"""
> diff --git a/gdb/testsuite/gdb.perf/lib/perftest/measure.py b/gdb/testsuite/gdb.perf/lib/perftest/measure.py
> new file mode 100644
> index 0000000..7478be5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/lib/perftest/measure.py
> @@ -0,0 +1,146 @@
> +# Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +import time
> +import os
> +import gc
> +
> +class Measure(object):
> + """A class that measure and collect the interesting data for a given testcase.
> +
> + An instance of Measure has a collection of measurements, and each
> + of them is to measure a given aspect, such as time and memory.
> + """
> +
> + def __init__(self, measurements):
> + """Constructor of measure.
> +
> + measurements is a collection of Measurement objects.
> + """
> +
> + self.measurements = measurements
> +
> + def measure(self, func, id):
> + """Measure the operations done by func with a collection of measurements."""
> + for m in self.measurements:
> + m.start(id)
> +
> + # Enable GC, force GC and disable GC before running test in order to reduce
> + # the interference from GC.
> + gc.enable()
> + gc.collect()
> + gc.disable()
> +
> + func()
> +
> + gc.enable()
> +
> + for m in self.measurements:
> + m.stop(id)
> +
> + def report(self, reporter, name):
> + """Report the measured results."""
> + for m in self.measurements:
> + m.report(reporter, name)
> +
> +class Measurement(object):
> + """A measurement for a certain aspect."""
> +
> + def __init__(self, name, result):
> + """Constructor of Measurement.
> +
> + Attribute result is the TestResult associated with measurement.
> + """
> + self.name = name;
> + self.result = result
> +
> + def start(self, id):
> + """Abstract method to start the measurement."""
> + raise NotImplementedError("Abstract Method:start")
> +
> + def stop(self, id):
> + """Abstract method to stop the measurement.
> +
> + When the measurement is stopped, we've got something, and
> + record them in result.
> + """
> + raise NotImplementedError("Abstract Method:stop.")
> +
> + def report(self, reporter, name):
> + """Report the measured data by argument reporter."""
> + self.result.report(reporter, name + " " + self.name)
> +
> +class MeasurementCPUTime(Measurement):
> + """Measurement on CPU time."""
> + # On UNIX, time.clock() measures the amount of CPU time that has
> + # been used by the current process. On Windows it will measure
> + # wall-clock seconds elapsed since the first call to the function.
> + # Something other than time.clock() should be used to measure CPU
> + # time on Windows.
> +
> + def __init__(self, result):
> + super(MeasurementCPUTime, self).__init__("cpu_time", result)
> + self.start_time = 0
> +
> + def start(self, id):
> + self.start_time = time.clock()
> +
> + def stop(self, id):
> + if os.name == 'nt':
> + cpu_time = 0
> + else:
> + cpu_time = time.clock() - self.start_time
> + self.result.record (id, cpu_time)
> +
> +class MeasurementWallTime(Measurement):
> + """Measurement on Wall time."""
> +
> + def __init__(self, result):
> + super(MeasurementWallTime, self).__init__("wall_time", result)
> + self.start_time = 0
> +
> + def start(self, id):
> + self.start_time = time.time()
> +
> + def stop(self, id):
> + wall_time = time.time() - self.start_time
> + self.result.record (id, wall_time)
> +
> +class MeasurementVmSize(Measurement):
> + """Measurement on memory usage represented by VmSize."""
> +
> + def __init__(self, result):
> + super(MeasurementVmSize, self).__init__("vmsize", result)
> +
> + def _compute_process_memory_usage(self, key):
> + file_path = "/proc/%d/status" % os.getpid()
> + try:
> + t = open(file_path)
> + v = t.read()
> + t.close()
> + except:
> + return 0
> + i = v.index(key)
> + v = v[i:].split(None, 3)
> + if len(v) < 3:
> + return 0
> + return int(v[1])
> +
> + def start(self, id):
> + foo = 0

Replace "foo = 0" with "pass".
I think that's the preferred way to do this.

> +
> + def stop(self, id):
> + memory_used = self._compute_process_memory_usage("VmSize:")
> + self.result.record (id, memory_used)
> diff --git a/gdb/testsuite/gdb.perf/lib/perftest/perftest.py b/gdb/testsuite/gdb.perf/lib/perftest/perftest.py
> new file mode 100644
> index 0000000..f7b8a8d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/lib/perftest/perftest.py
> @@ -0,0 +1,72 @@
> +# Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +import testresult
> +import reporter
> +from measure import Measure
> +from measure import MeasurementCPUTime
> +from measure import MeasurementWallTime
> +from measure import MeasurementVmSize

I don't have a strong preference, but it's odd the "CPU" is all caps
and "Vm" isn't.

> +
> +class TestCase(object):
> + """Base class of all performance testing cases.
> +
> + Each sub-class should override methods execute_test, in which
> + several GDB operations are performed and measured by attribute
> + measure. Sub-class can also override method warm_up optionally
> + if the test case needs warm up.
> + """
> +
> + def __init__(self, name, measure):
> + """Constructor of TestCase.
> +
> + Construct an instance of TestCase with a name and a measure
> + which is to measure the test by several different measurements.
> + """
> +
> + self.name = name
> + self.measure = measure
> +
> + def execute_test(self):
> + """Abstract method to do the actual tests."""
> + raise NotImplementedError("Abstract Method.")
> +
> + def warm_up(self):
> + """Do some operations to warm up the environment."""

I"m not sure what the conventions are for an empty function with a doc string.
Do we need to add "pass" here?

> +
> + def run(self, warm_up=True, append=True):
> + """Run this test case.
> +
> + It is a template method to invoke method warm_up,
> + execute_test, and finally report the measured results.
> + If parameter warm_up is True, run method warm_up. If parameter
> + append is True, the test result will be appended instead of
> + overwritten.
> + """
> + if warm_up:
> + self.warm_up()
> +
> + self.execute_test()
> + self.measure.report(reporter.TextReporter(append), self.name)
> +
> +class TestCaseWithBasicMeasurements(TestCase):
> + """Test case measuring CPU time, wall time and memory usage."""
> +
> + def __init__(self, name):
> + result_factory = testresult.SingleStatisticResultFactory()
> + measurements = [MeasurementCPUTime(result_factory.create_result()),
> + MeasurementWallTime(result_factory.create_result()),
> + MeasurementVmSize(result_factory.create_result())]
> + super (TestCaseWithBasicMeasurements, self).__init__ (name, Measure(measurements))
> diff --git a/gdb/testsuite/gdb.perf/lib/perftest/reporter.py b/gdb/testsuite/gdb.perf/lib/perftest/reporter.py
> new file mode 100644
> index 0000000..a902e4b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/lib/perftest/reporter.py
> @@ -0,0 +1,64 @@
> +# Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +class Reporter(object):
> + """Base class of reporter to report test results in a certain format.
> +
> + Subclass, which is specific to a report format, should overwrite
> + methods report, start and end.
> + """
> +
> + def __init__(self, append):
> + """Constructor of Reporter.
> +
> + attribute append is used to determine whether to append or
> + overwrite log file.
> + """
> + self.append = append
> +
> + def report(self, *args):
> + raise NotImplementedError("Abstract Method:report.")
> +
> + def start(self):
> + """Invoked when reporting is started."""
> + raise NotImplementedError("Abstract Method:start.")
> +
> + def end(self):
> + """Invoked when reporting is done.
> +
> + It must be overridden to do some cleanups, such as closing file
> + descriptors.
> + """
> + raise NotImplementedError("Abstract Method:end.")
> +
> +class TextReporter(Reporter):
> + """Report results in a plain text file 'perftest.log'."""
> +
> + def __init__(self, append):
> + super (TextReporter, self).__init__(Reporter(append))
> + self.txt_log = None
> +
> + def report(self, *args):
> + self.txt_log.write(' '.join(str(arg) for arg in args))
> + self.txt_log.write('\n')
> +
> + def start(self):
> + if self.append:
> + self.txt_log = open ("perftest.log", 'a+');
> + else:
> + self.txt_log = open ("perftest.log", 'w');
> +
> + def end(self):
> + self.txt_log.close ()
> diff --git a/gdb/testsuite/gdb.perf/lib/perftest/testresult.py b/gdb/testsuite/gdb.perf/lib/perftest/testresult.py
> new file mode 100644
> index 0000000..e571f12
> --- /dev/null
> +++ b/gdb/testsuite/gdb.perf/lib/perftest/testresult.py
> @@ -0,0 +1,57 @@
> +# Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +class TestResult(object):
> + """Base class to record and report test results.
> +
> + Method record is to record the results of test case, and report
> + method is to report the recorded results by a given reporter.
> + """
> +
> + def record(self, parameter, result):
> + raise NotImplementedError("Abstract Method:record.")
> +
> + def report(self, reporter, name):
> + """Report the test results by reporter."""
> + raise NotImplementedError("Abstract Method:report.")
> +
> +class SingleStatisticTestResult(TestResult):
> + """Test results for the test case with a single statistic."""
> +
> + def __init__(self):
> + super (SingleStatisticTestResult, self).__init__ ()
> + self.results = dict ()
> +
> + def record(self, parameter, result):
> + self.results[parameter] = result
> +
> + def report(self, reporter, name):
> + reporter.start()
> + for key in sorted(self.results.iterkeys()):
> + reporter.report(name, key, self.results[key])
> + reporter.end()
> +
> +class ResultFactory(object):
> + """A factory to create an instance of TestResult."""
> +
> + def create_result(self):
> + """Create an instance of TestResult."""
> + raise NotImplementedError("Abstract Method:create_result.")
> +
> +class SingleStatisticResultFactory(ResultFactory):
> + """A factory to create an instance of SingleStatisticTestResult."""
> +
> + def create_result(self):
> + return SingleStatisticTestResult()
> diff --git a/gdb/testsuite/lib/perftest.exp b/gdb/testsuite/lib/perftest.exp
> new file mode 100644
> index 0000000..59b204d
> --- /dev/null
> +++ b/gdb/testsuite/lib/perftest.exp
> @@ -0,0 +1,148 @@
> +# Copyright (C) 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +namespace eval PerfTest {
> + # The name of python file on build.
> + variable remote_python_file
> +
> + # The source files are compiled successfully or not.
> + variable compiled_ok
> +
> + # A private method to set up GDB for performance testing.
> + proc _setup_perftest {} {
> + variable remote_python_file
> + global srcdir subdir testfile
> +
> + set remote_python_file [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
> +
> + # Set sys.path for module perftest.
> + gdb_test_no_output "python import os, sys"
> + gdb_test_no_output "python sys.path.insert\(0, os.path.abspath\(\"${srcdir}/${subdir}/lib\"\)\)"
> + gdb_test_no_output "python exec (open ('${remote_python_file}').read ())"
> + }
> +
> + # A private method to do some cleanups when performance test is
> + # finished.
> + proc _teardown_perftest {} {
> + variable remote_python_file
> +
> + remote_file host delete $remote_python_file
> + }
> +
> + # Compile source files of test case. BODY is the tcl code to do
> + # actual compilation and it should invoke 'PerfTest::compiled' if
> + # compilation is successful.
> + proc compile {body} {
> + global GDB_PERFORMANCE
> +
> + if { [info exists GDB_PERFORMANCE]
> + && [string compare $GDB_PERFORMANCE "run"] } {
> + variable compiled_ok
> +
> + set compiled_ok 0
> + uplevel 2 $body
> +
> + if {!$compiled_ok} {
> + untested "Could not compile source files."
> + return -1
> + }
> + }
> + }
> +
> + # Mark the compilation is finished successfully.
> + proc compiled {} {
> + variable compiled_ok
> +
> + set compiled_ok 1
> + }
> +
> + # Start up GDB.
> + proc startup_gdb {body} {
> + variable compiled_ok
> +
> + if {!$compiled_ok} {
> + return
> + }
> +
> + uplevel 2 $body
> + }
> +
> + # Run the performance test.
> + proc run {body} {
> + global timeout
> + global GDB_PERFORMANCE_TIMEOUT
> +
> + set oldtimeout $timeout
> + if { [info exists GDB_PERFORMANCE_TIMEOUT] } {
> + set timeout $GDB_PERFORMANCE_TIMEOUT
> + } else {
> + set timeout 3000
> + }
> + uplevel 2 $body
> +
> + set timeout $oldtimeout
> + }
> +
> + # The top-level interface to PerfTest.
> + # COMPILE is the tcl code to generate and compile source files.
> + # STARTUP is the tcl code to start up GDB.
> + # RUN is the tcl code to drive GDB to do some operations.
> + proc assemble {compile startup run} {
> + variable compiled_ok
> + global GDB_PERFORMANCE
> +
> + set compiled_ok 1
> + eval $compile
> +
> + if {!$compiled_ok} {
> + return
> + }
> +
> + # Don't execute the run if GDB_PERFORMANCE=compile.
> + if { [info exists GDB_PERFORMANCE]
> + && [string compare $GDB_PERFORMANCE "compile"] == 0} {
> + return
> + }
> +
> + eval $startup
> +
> + _setup_perftest
> +
> + eval $run
> +
> + _teardown_perftest
> + }
> +}
> +
> +# Return true if performance tests are skipped.
> +
> +proc skip_perf_tests { } {
> + global GDB_PERFORMANCE
> +
> + if [info exists GDB_PERFORMANCE] {
> +
> + if { [string compare $GDB_PERFORMANCE "compile"]
> + && [string compare $GDB_PERFORMANCE "run"]
> + && [string compare $GDB_PERFORMANCE "both"] } {

How about just "$GDB_PERFORMANCE" != "compile", etc. ?

> + # GDB_PERFORMANCE=compile|run|both is allowed.
> + unsupported "Unknown value of GDB_PERFORMANCE."
> + return 1
> + }
> +
> + return 0
> + }
> +
> + return 1
> +}
> --
> 1.7.7.6
>