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

RESOLVED FIXED

Status

Release Engineering
General
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: karlt, Assigned: alice)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
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.
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.
(Assignee)

Comment 2

10 years ago
Created attachment 343346 [details] [diff] [review]
[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.
Assignee: nobody → anodelman
Attachment #343346 - Flags: review?(nthomas)
(Assignee)

Comment 3

10 years ago
See bug 387395.
Attachment #343346 - Flags: review?(nthomas) → review+
Comment on attachment 343346 [details] [diff] [review]
[Checked in]fix median based upon correct version in framecycler.js

Looks good to me.
(Assignee)

Comment 5

10 years ago
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
Attachment #343346 - Attachment description: fix median based upon correct version in framecycler.js → [Checked in]fix median based upon correct version in framecycler.js
(Assignee)

Comment 6

10 years ago
Change pushed to production.

Will have to rev standalone talos for this to end up in the zip package.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED

Updated

9 years ago
Component: Release Engineering: Talos → Release Engineering
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.