[Webkit-unassigned] [Bug 188425] Added comparision for individual iterations on analysis task page.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188425

--- Comment #5 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 346818
  --> https://bugs.webkit.org/attachment.cgi?id=346818
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=346818&action=review

> Websites/perf.webkit.org/ChangeLog:10
> +        Refactored t-distribution inverse lookup to any degree of freedom with 4 digit precision.

4 digit precision -> 5 significant figures.

> Websites/perf.webkit.org/public/shared/statistics.js:37
> +        for (var probability in tDistributionTable)

Use const?

> Websites/perf.webkit.org/public/shared/statistics.js:71
> +    function sampleMeanAndVarianceForAggregatedInfo(statisticalInfo) {

This is not a descriptive name. "info" doesn't provide any meaningful semantics and "Aggregated"
makes it sound like this is the one that computes statistics of the means since mean is an aggregated value.
In general, we should avoid vague variable or argument names such as "statisticalInfo"

How about sampleMeanAndVarianceFromMultipleSamples? since I think what's important here is that
we're trying to compute the mean & variance of multiple sets of samples.

> Websites/perf.webkit.org/public/shared/statistics.js:108
> +        for (var probability in tDistributionTable) {

Use const?

> Websites/perf.webkit.org/public/shared/statistics.js:295
> +            let mid = low + parseInt((high - low) / 2);
> +            const entry = tDistributionTableForProbability[mid];

This binary search isn't quite right. We need to be binary-searching based on ends, not the index of each entry.

> Websites/perf.webkit.org/public/shared/statistics.js:305
> +    const tDistributionTable = {

I don't think we should rename this table.
It's important to know whether these t-distribution values are one-sided or two-sided values.

> Websites/perf.webkit.org/public/shared/statistics.js:307
> +            {end: 1, value: 3.0777},    {end: 2, value: 1.8856},    {end: 3, value: 1.6377},    {end: 4, value: 1.5332},

For values that only occupy a single slot, I think we're better off omitting the number.
so, that we can see it like:
3.0777, 1.8856, 1.6377, 1.5332
for the first hundred values.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180822/bc01ac37/attachment.html>