Re: [PATCH 2/3] Perf test framework

Yao Qi
2013-10-10T03:04:48Z

On 10/09/2013 02:37 PM, Doug Evans wrote:
> I'm not sure what your plans are, but another thing we need to measure
> is memory usage.

I plan to add memory measurement later. Current framework supports
multiple measurements in a single test case.

class SolibLoadUnload(perftest.SingleStatisticTestCase):
def __init__(self, solib_number):
# We want to measure time in this test.
super (SolibLoadUnload, self).__init__ ([measure.Measurement.TIME],
"solib")

Once we want to measure memory usage, we can modify it like:

class SolibLoadUnload(perftest.SingleStatisticTestCase):
def __init__(self, solib_number):
# We want to measure time in this test.
super (SolibLoadUnload, self).__init__ ([measure.Measurement.TIME,
measure.Measurement.MEMORY],
"solib")
> There are other things too, but time (both cpu and wall) and memory
> usage are a good start.
> Most basic performance tests might want all three (e.g., why run a
> test three times, it's all good data).

All data needed by each measurement can be collected in one run.

> How will MultiStatistic tests work?

All our test cases are SingleStatistic because they are quite simple,
so we don't think carefully how MultiStatistic tests work. I'd like to
consider this when we really need a MultiStatistic 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 exists.
> For my uses, I need to be able to do the compilation and skip running
> the tests (the tests will be run in a separate step).
> Just a suggestion, but how about GDB_PERFORMANCE=yes|compile|run
> [or whatever spellings work - just trying to reduce the number of variables]
>

That makes sense to me.

>> >+
>> >+class Measurement(object):
>> >+ """A measurement for a certain aspect."""
>> >+
>> >+ # enum of each measurement.
>> >+ TIME = 1
>> >+
>> >+ @classmethod
>> >+ def create_instance(cls, id, result_factory):
>> >+ """A factory method to create an instance of Measurement subclass
>> >+ according to id."""
>> >+ if id == Measurement.TIME:
>> >+ return MeasurementTime(result_factory.create_result())
>> >+ else:
>> >+ raise RuntimeError("Unknown type of measurement.")
> I don't know factories that well, but IWBN if every measurement kind
> didn't need an entry here.
>

In each test case, the enum of Measurement are specified in a
collection, like,

[measure.Measurement.CPU_TIME, measure.Measurement.WALL_TIME,
measure.Measurement.MEMORY]

the static method create_instance creates the instance of Measurement
sub-classes, which looks reasonable to me.

Without this static method, each test case has to create a collection
of Measurement instances for its own.

>> >+
>> >+class MeasurementTime(Measurement):
> "Time" is ambiguous. wall time or cpu time?
> I'd rename this to make it explicit what kind is being measured (in
> the class name and its output).
>

Agreed.

>> >+
>> >+import testresult
>> >+import reporter
>> >+from measure import Measure
>> >+
>> >+class TestCase(object):
> PerfTestCase?
>

I'd like to leave "TestCase" as-is because they have been in package
perftest.

>> >+ """
>> >+ 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.
>> >+
> It's odd to have a blank line here. I couldn't find anything
> definitive in pep257 though.
>
>> >+ """
> IIUC, pep257 says have a blank line here.
>

I was confused by this sentence "Docstrings documenting functions or
methods generally don't have this requirement, unless the function or
method's body is written as a number of blank-line separated sections
-- in this case, treat the docstring as another section, and precede it
with a blank line".

and follow the example in pep-0257

def complex(real=0.0, imag=0.0):
"""Form a complex number.

Keyword arguments:
real -- the real part (default 0.0)
imag -- the imaginary part (default 0.0)

"""
if imag == 0.0 and real == 0.0: return complex_zero
...

>> >+
>> >+ # Run the performance test.
>> >+ proc run {body} {
>> >+ global timeout
>> >+
>> >+ set timeout 3000
> Probably want to parameterize the timeout.
>

OK.

>> >+ uplevel 2 $body
>> >+ }
>> >+
>> >+ # The top-level interface to PerfTest.
>> >+ # PREPARE is the tcl code to prepare for the performance testing,
>> >+ # including generating and compiling source files, starting up GDB.
>> >+ # RUN is the tcl code to drive GDB to do some operations.
>> >+ proc assemble {prepare run} {
> According to this comment "prepare" combines compiling source files
> with starting up gdb.
> Can we separate them?

OK.

> OTOH, there's the call to _setup_gdb after the "eval $prepare".
> It's a bit confusing.
>

What is confusing? the name "_setup_gdb"? $prepare is to compile
sources and start up gdb, and proc _setup_gdb is to set up perf test in
a running gdb. How about rename "_setup_gdb" with "_setup" or
"_setup_perftest"?

--
Yao (é½å°§)