[llvm-commits] [PATCH][LNT] Add support for selection of timing statistic (user or real time)

[Daniel Dunbar]

On Wed, Aug 15, 2012 at 10:57 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Wed, Aug 15, 2012 at 10:34 AM, Daniel Dunbar <daniel at zuster.org> wrote:
>> Hi David,
>>
>> Sorry for review lag, I've tried to fix my filters so I see this stuff sooner.
>
> Thanks
>
>> The LNT side of the patch looks fine, but I think the test-suite side
>> would be substantially simplified if you just added an option to
>> timeit-target to report the real time instead of the user time as the
>> "program time". This would avoid needing most of the other changes.
>
> It would reduce the number of changes, but I think it would
> complicate, rather than simplify, the test-suite (which is already way
> more complicated than it should be due to similar "smallest change"
> rather than "simplest post-patch state" tradeoffs, it seems to me).
> That option did cross my mind - honestly at this point I forget
> whether I tried that or how far I got down the path before I worked on
> this version.
>
> Part of the point was to provide more information up to reports & then
> let reports decide how to format that data - or even to report
> multiple statistics to LNT & then we could investigate combinations or
> selections of data points we might find interesting.

Ok. In that case, I'm fine with the patch.

>
>> Also, I can't imagine how your patch actually works given that it
>> deletes the --summary option, but the test suite still uses it?
>
> That one's easy: 20 lines up there's exactly the same 9 lines of code.
> This is just removing some weird copy-paste redundancy, so far as I
> can tell.

Oops, thanks then!

 - Daniel