459598 - "medians" for individual pages in Tp don't seem to be medians

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Karl Tomlinson (:karlt)]

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1223561452.1223564931.19544.gz

NOISE: |i|pagename|median|mean|min|max|runs|
NOISE: |0;www.yahoo.com/www.yahoo.com/index.html;1185;97.88888888888889;89;1185;1185;104;98;90;95;108;89;93;100;104

iiuc, mean=97.88888888888889, min=89, max=1185

The figure reported on the "details" graph

http://graphs-stage.mozilla.org/graph.html#type=series&show=409924

is 1185, which appears to come from the "median" column but it is not a median.

The same issue for another log:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1223805652.1223809129.9800.gz

NOISE: |355;www.tv.com/www.tv.com/index.html;1695;976.2222222222222;811;1695;1346;863;1074;852;815;811;1032;1179;814;1695

The number reported on the details graph is the one in the "median" column but it is not a median.

I can't work out where the number in the "median" column comes from.  It is not usually the maximum, but doesn't seem to usually be a median either.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Alice gave me some pointers here, so what I think is happening is that 
 http://mxr.mozilla.org/mozilla/source/layout/tools/pageloader/report.js#93
is an older, wonkier, version of the code. In particular 
* line 112 does an alphabetical sort instead of a numerical one, so you get things like 1,1185,2,... instead of 1,2,1185. That mucks up popping off the largest result from the array, and picking out the median by index
* When the array is of even length we take the mean of the two middle values, but the indices should be n-1, n because they are zero-based

Another implementation (now unused)
  http://mxr.mozilla.org/mozilla/source//testing/performance/talos/page_load_test/framecycler.html?raw=1
seems to do this stuff already, eg compareNumbers(), so it might be worth trawling for other differences.

Comment 2

[alice nodelman [:alice] [:anode]]

Attachment: [Checked in]fix median based upon correct version in framecycler.js

These fixes already ended up in framecycler.js - they need to be matched up on report.js.

Comment 3

[alice nodelman [:alice] [:anode]]

See bug 387395.

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 343346 [details] [diff] [review]
[Checked in]fix median based upon correct version in framecycler.js

Looks good to me.

Comment 5

[alice nodelman [:alice] [:anode]]

Comment on attachment 343346 [details] [diff] [review]
[Checked in]fix median based upon correct version in framecycler.js

Checking in report.js;
/cvsroot/mozilla/layout/tools/pageloader/report.js,v  <--  report.js
new revision: 1.9; previous revision: 1.8
done

Comment 6

[alice nodelman [:alice] [:anode]]

Change pushed to production.

Will have to rev standalone talos for this to end up in the zip package.