[PATCH] performance history specification

Robert Collins robertc at robertcollins.net
Fri Jul 14 05:09:45 BST 2006


On Wed, 2006-07-12 at 14:29 -0500, John Arbash Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Jan Balster wrote:
> > Hi all,
> > 
> > at europython Robert Collins, Carl Friedrich Bolz and Holger Krekel started a
> > branch to implement the performance history specification
> > (https://launchpad.net/products/bzr/+spec/performance-history).
> > The branch is located at codespeak.net:
> > http://codespeak.net/bzr/branches/bzr.performance-history
> > 
> > Robert suggested to merge the changes into the main branch, so here is the
> > official request, the bundle is attached.
> > 
> > Cheers,
> >        Jan
> > 
> 
> ...
> 
> > -    def __init__(self, stream, descriptions, verbosity, pb=None):
> > +    def __init__(self, stream, descriptions, verbosity, pb=None,
> > +                 bench_history=None):
> >          unittest._TextTestResult.__init__(self, stream, descriptions, verbosity)
> >          self.pb = pb
> > +        if bench_history is not None:
> > +            bench_history.write("--date %s\n" % time.time())
> > +        self._bench_history = bench_history
> 
> Seems like you are writing a timestamp from epoch time here, rather than
> a date. It would look nice if you wrote:
> 
> bench_history.write(time.strftime('--date %Y-%m-%d %H:%M:%S\n'))
> 
> Or if you want something more official, maybe:
> bench_history.write('--date %s\n' %
> 		    (datetime.datetime.utcnow().isoformat(),))
> 
> But in general, if it is for human viewing time.time() is not very good.

The file is not for human viewing - its a log file for data gathering.
time.time() gives seconds since epoch, in UTC, and can be trivially
formatted into any date when the file is parsed.

> ...
> 
> 
> v-- minor grammer fix
>         vv
> we want to profile a call
> 
> > +        # we want profile a call and check that its test duration is recorded
> > +        # make a new test instance that when run will generate a benchmark
> > +        example_test_case = TestTestResult("_time_hello_world_encoding")
> > +        # execute the test, which should succeed and record times
> > +        example_test_case.run(result)
> > +        lines = result_stream.getvalue().splitlines()
> > +        self.assertEqual(2, len(lines))
> > +        self.assertContainsRe(lines[1],
> > +            " *[0-9]+ms bzrlib.tests.test_selftest.TestTestResult"
> > +            "._time_hello_world_encoding")
> > + 
> 
> It looks pretty good. The only thing it isn't doing is checking to make
> sure .perf-history isn't being overwritten.
> Also, it wasn't clear where it was creating the .perf-history file. I
> assume you were shooting for putting it inside the project (not the test
> temp dirs)
> Which means that .bzrignore should be updated to ignore the newly
> generated file.

Good point, +1 on adding an ignore to .bzrignore for .perf-history.

Yes, it puts it in cwd, which is what the tests test. And you are right,
we don't test that it appends always. That should be a TODO for the next
set of performance work related to this.

That said John, are you happy for this to merge as is (with .bzrignore
changed)?

Cheers,
Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060714/dddfd162/attachment.pgp 


More information about the bazaar mailing list