Bug 97960

Summary:

Perf. results page should have heuristics to detect bimodal distributions

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

Tools / Tests

Assignee:

Ryosuke Niwa <rniwa>

Status:

NEW

Severity:

Normal

CC:

abarth, eric, haraken, morrita, zoltan

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

77037

Attachments:

Description	Flags
Adds the feature	benjamin: review-
Sample output	none

Ryosuke Niwa

Reported 2012-09-28 18:35:44 PDT

We should auto-detect tests with bi-modal distributions just like we detect tests with time-dependent distributions. We can implement a simple heuristic based on binning (separate values into bins and look for two modes separated by reasonably big space).

Attachments
Adds the feature (6.75 KB, patch) 2012-09-28 18:42 PDT, Ryosuke Niwa	benjamin: review-	Details Formatted Diff Diff
Sample output (97.37 KB, text/html) 2012-09-28 18:43 PDT, Ryosuke Niwa	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Eric Seidel (no email)

Comment 1 2012-09-28 18:36:59 PDT

Sounds cool. I didn't realize we already did "time dependent distribution" detection.

Ryosuke Niwa

Comment 2 2012-09-28 18:42:43 PDT

Created attachment 166345 [details] Adds the feature

Ryosuke Niwa

Comment 3 2012-09-28 18:43:35 PDT

Created attachment 166346 [details] Sample output

Ryosuke Niwa

Comment 4 2012-09-28 18:50:22 PDT

(In reply to comment #1) > Sounds cool. I didn't realize we already did "time dependent distribution" detection. Yeah, we just added that thing a couple of days ago.

Ryosuke Niwa

Comment 5 2012-10-01 10:09:26 PDT

Any reviewer?

Zoltan Horvath

Comment 6 2012-10-04 14:38:05 PDT

Comment on attachment 166345 [details] Adds the feature I reviewed the change and the sample and it looks okay for me.

Benjamin Poulain

Comment 7 2013-02-26 14:52:52 PST

Comment on attachment 166345 [details] Adds the feature View in context: https://bugs.webkit.org/attachment.cgi?id=166345&action=review r- because of the typo :-D > PerformanceTests/resources/results-template.html:420 > +function isHeuristicallyBiModal(values, expecetedNumberOfPointsInBin) { expecetedNumberOfPointsInBin ->expected. I am not sure why this is an argument here. > PerformanceTests/resources/results-template.html:426 > + var bins = []; var bins = new Array(numberOfBins) ? > PerformanceTests/resources/results-template.html:433 > + bins[Math.floor((values[i] - min) / binSize)] += 1 / values.length; Why "1 / values.length"? Wouldn't it be easier to follow by doing comparison with the sample size? E.g. bins[i] >= values.length / 4 > PerformanceTests/resources/results-template.html:452 > + if (firstMode + Math.max(1, numberOfBins / 5) >= i) > + continue; // Too close. > + var thereAreEnoughValuesBetweenModes = false; > + for (var j = firstMode + 1; j < i; j++) { > + if (bins[j] >= 0.1) { > + thereAreEnoughValuesBetweenModes = true; > + break; > + } > + } > + if (thereAreEnoughValuesBetweenModes) > + continue; > + return true; I am not sure I like this, it makes impossible to interpret what is being computed. Could this be achieved by having a bigger initial expecetedNumberOfPointsInBin, and only checking (i - firstMode) > threshold?

Note You need to log in before you can comment on or make changes to this bug.