Code review comment for lp://qastaging/~glcompbench-dev/glcompbench/manmower-options-3

Revision history for this message
Derek Foreman (derek-foreman) wrote :

> Based on your patch I have implemented what is essentially a more generic
> version of your proposal:
>
> https://code.launchpad.net/~linaro-graphics-wg/glcompbench/profiler-stat-
> collection

Very cool :)

> The current code in the branch is not complete yet. It only tracks per-
> iteration statistics, but adding per-test and total statistics is easy. The
> code is also missing the 'squares' stats datum, but this is trivial to add,
> too.
>
> In your proposal the total stats are displayed after each test, but they are
> not really per-test stats, which I find somewhat confusing. I think it would
> be better to show per-test stats after each test is completed and total stats
> only once at the end.

Yes, that sounds quite sensible.

> Also, I would rather keep the accumulation logic outside of the profiler
> class. It's easy to calculate accumulated data outside the profiler, given the
> statistical data in the 'total' collection (calculating std. dev. needs a bit
> more care).
>
> What do you think?

It's not immediately obvious to me how I'd calculate std. dev. anymore. I could easily calculate the std. dev. of individual profiler points, but since there aren't the same number of each profiler point in an iteration, I can't figure out how to calculate std. dev. of test iterations, which is the more useful stat from my perspective.

Though perhaps there's a way that I'm just not seeing?

> Meanwhile I will merge into trunk some of the other changes that don't depend
> on the substats infrastructure.

Thanks!

« Back to merge proposal