This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/4] Perf test framework


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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]