Summary page should support calculating summary using weighted mean.
Created attachment 439301 [details] Patch
rdar://83554204
Comment on attachment 439301 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439301&action=review > Websites/perf.webkit.org/public/shared/statistics.js:19 > + this.weightedMean = function (values) { It's confusing to call this argument "values". Maybe valuesWithWeights? > Websites/perf.webkit.org/public/shared/statistics.js:27 > + if (!Array.isArray(item)) { > + totalWeight += 1; > + sum += item; > + continue; > + } I don't think we should do polymorphism like this. > Websites/perf.webkit.org/public/shared/statistics.js:31 > + const [value, weight] = item; > + totalWeight += weight; > + sum += value * weight; Why not just use a dictionary / object? e.g. totalWeight += item.weight; sum += item.weight * item.value; > Websites/perf.webkit.org/public/v3/pages/summary-page.js:17 > + this._customWeightConfigurations = summarySettings.customWeightConfigurations; It doesn't seem useful to say this is custom weight? How about just weightedConfigurations? > Websites/perf.webkit.org/public/v3/pages/summary-page.js:352 > + if (typeof weightForPlatform === 'number') There is no point in using === here. The value of typeof is always a string. But it seems problematic to assume anything that's not a number is an object? e.g. string and null.
Created attachment 439318 [details] Patch
Comment on attachment 439318 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=439318&action=review We really need tests for summary page... > Websites/perf.webkit.org/public/v3/pages/summary-page.js:17 > + this._weightConfigurations = summarySettings.weightConfigurations; This sounds like the configuration of weights. So I think it should be weightedConfigurations. Like excluded configurations, we're listing a list of configurations whose weight isn't 1 along with its weight. > Websites/perf.webkit.org/public/v3/pages/summary-page.js:354 > + const isNumber = (object) => ['number', 'string'].includes(typeof object) && !isNaN(+object); > + if (isNumber(weightForPlatform)) > + return +weightForPlatform; This isn't quite right. This will throw an exception if weightForPlatform is null or true/false as well. I think what we want to check instead is simply isNaN(+object). i.e. if (!isNaN(weightForPlatform)) return weightForPlatform; > Websites/perf.webkit.org/public/v3/pages/summary-page.js:358 > + const weight = weightForPlatform[metricId] > + if (isNumber(weight)) > + return +weight; Here, we probably want to do instead: if (typeof weightForPlatform != 'object' || isNaN(+weightForPlatform[metricId])) return 1;
Landed in r283153.