Last Comment Bug 750507 - Need a talos jank test
: Need a talos jank test
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: -- normal (vote)
: Firefox 15
Assigned To: Kartikaya Gupta (email:kats@mozilla.com)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-30 14:46 PDT by Kartikaya Gupta (email:kats@mozilla.com)
Modified: 2012-05-04 08:52 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Patch (2.39 KB, patch)
2012-05-02 06:32 PDT, Kartikaya Gupta (email:kats@mozilla.com)
jmuizelaar: review+
Details | Diff | Splinter Review

Description Kartikaya Gupta (email:kats@mozilla.com) 2012-04-30 14:46:56 PDT
We want a talos test to measure jank. Sort of like testPan (aka robopan) does now, but somewhat enhanced so we weight missed frames by the amount we missed the frame by, rather than just counting the number of missed frames.

:jmaher, should we just change the calculation used in testPan, or is it better to write a new test for this?
Comment 1 Joel Maher ( :jmaher) 2012-05-01 06:17:19 PDT
I am fine just changing testPan.  From the description of this issue it seems like we are not getting all we want from testPan and adjusting it would give us what we want.
Comment 2 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-01 09:11:28 PDT
I put together a first patch and ran it on try a bunch of times, but it's giving very variable results, just like the new tcheckerboard and tcheckerboard2 numbers. See https://tbpl.mozilla.org/?tree=Try&rev=868d2f723523 for the patch and numbers. I'm going to try dividing by number of frames to hopefully smooth out the numbers a bit. It should still be a measure of jank but with less variability because it won't depend on how long the test runs for.
Comment 3 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-02 06:32:31 PDT
Created attachment 620288 [details] [diff] [review]
Patch

The patch with division by number of frames didn't reduce variability much, I'm not sure why. The results for that can be found at https://tbpl.mozilla.org/?tree=Try&rev=fc96c85bee42

This is just the first patch I tried, where we sum the squares of the missed frame times in milliseconds.
Comment 4 Jeff Muizelaar [:jrmuizel] 2012-05-03 11:09:33 PDT
Comment on attachment 620288 [details] [diff] [review]
Patch

Review of attachment 620288 [details] [diff] [review]:
-----------------------------------------------------------------

Might be worth adding a note about why we're choosing this new metric over the old one.

::: build/mobile/robocop/FennecNativeDriver.java.in
@@ +235,4 @@
>              for (int i = 1; i < frames.size(); i++) {
> +                long frameTime = frames.get(i) - frames.get(i - 1);
> +                if (frameTime > FRAME_TIME_THRESHOLD) {
> +                    badness += Math.pow(frameTime - FRAME_TIME_THRESHOLD, 2);

I would square this by hand instead of using pow.

@@ +245,5 @@
>              log(LogLevel.ERROR, e);
>          }
>  
> +        // higher values are worse, and the test failing is the worst!
> +        return 12345678;

MAX_INT?
Comment 5 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-03 12:45:15 PDT
Landed with review comments addressed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ce39c695793
Comment 6 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-04 08:52:42 PDT
https://hg.mozilla.org/mozilla-central/rev/6ce39c695793

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