On Thu, 02 Jul 2015 17:45:13 +0100 "Ben Avison" <bavi...@riscosopen.org> wrote:
> On Wed, 10 Jun 2015 14:32:49 +0100, Pekka Paalanen <ppaala...@gmail.com> > wrote: > > most of the patches are trivial cleanups. The meat are the last two: > > CSV output mode and skipping the memory speed benchmark. > > > > Both new features are designed for an external benchmarking harness, > > that runs several different versions of Pixman with lowlevel-blt-bench > > in an alternating fashion. Alternating iterations are needed to get > > reliable results on platforms like the Raspberry Pi. > > These look like sensible improvements to me. A few minor points: > > Patch 2 commit message: > > Move explanation printing to a new file. This will help with > ^^^^ function Fixed. > Patch 8: if the aim is machine readability, I'd suggest no space after > the comma. Otherwise if comma is used as a field separator, the first > field has no leading space but all the others do, which may make things a > little more fiddly when post-processing the results with some tools. Done. > Not really your fault, but I noticed it when trying out the new versions: > it doesn't fault an attempt to list more than one pattern on the command > line. It just acts on the last one. It should really either fault it, or > (more usefully) benchmark all of the patterns specified. This would allow > a subset of the "all" tests to be performed, sharing the same memcpy > measurement overhead, or a group of operations including those not in the > "all" list. I will make a follow-up that at least refuses to run with extra arguments. The runner script I am using would not be able to use multiple patterns on a single execution, and it would just need to split the output anyway. In a multi-pattern case, it would also be useful to record the pattern too, so that would be another type of CSV output for different scripts to parse. > In other respects, happy to give my Reviewed-by:. Cool, and since no other comments in a month, pushed: 2be523b..e2d211a master -> master Thanks, pq _______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/pixman