751158 - Create tcheckboard3 to measure checkerboard with low res off

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[JP Rosevear [:jpr]]

Create tcheckboard3 to measure checkerboard with low res off.  Should be identical to tcheckerboard2 other than this.  Will ensure that low res is not causing any testing problems on tcheckerboard2.  Also will tell us how much low res impacts us.

Comment 1

[Joel Maher ( :jmaher ) (UTC -0800)]

any chance we can get rid of tcheckerboard (original)?  My concern is these tests take a long time and eat up our limited resources.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

(In reply to JP Rosevear [:jpr] from comment #0)
> Create tcheckboard3 to measure checkerboard with low res off.  Should be
> identical to tcheckerboard2 other than this.

tcheckerboard2 already doesn't take into account the low-res screenshot. It uses the programmatic measure of checkerboarding, which doesn't include retained tiles or low-res screenshot, only actual valid areas.

Comment 3

[JP Rosevear [:jpr]]

(In reply to Kartikaya Gupta (:kats) from comment #2)
> (In reply to JP Rosevear [:jpr] from comment #0)
> > Create tcheckboard3 to measure checkerboard with low res off.  Should be
> > identical to tcheckerboard2 other than this.
> 
> tcheckerboard2 already doesn't take into account the low-res screenshot. It
> uses the programmatic measure of checkerboarding, which doesn't include
> retained tiles or low-res screenshot, only actual valid areas.

Understood.

According to discussion in IRC just now however, preffing off the screenshot doesn't actually stop taking the screenshot, it just stops displaying it.  We need to fix that in order to make this test (fixing this should actually give us less checkboarding).

Comment 4

[Geoff Brown [:gbrown]]

Attachment: a preliminary attempt

I am not at all confident that I understand what is needed here ... hoping that a patch will clarify. 

kats -- am I on the right track?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 621189 [details] [diff] [review]
a preliminary attempt

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

This looks good to me! Just a few nits.

::: mobile/android/base/GeckoAppShell.java
@@ +2245,5 @@
>      private static int clamp(int min, int val, int max) {
>          return Math.max(Math.min(max, val), min);
>      }
>  
> +    public static void disableScreenshot() {

Add a comment saying this is invoked by reflection from robocop tests.

::: mobile/android/base/tests/testCheck3.java.in
@@ +46,5 @@
> +        System.out.println("Completeness score: " + completeness);
> +        long msecs = System.currentTimeMillis();
> +        mAsserter.dumpLog("__startTimestamp" + msecs + "__endTimestamp");
> +    }
> +    private void disableScreenshot() {

nit: add a blank line between the two functions

@@ +52,5 @@
> +            ClassLoader classLoader = getActivity().getClassLoader();
> +            Class appshell = classLoader.loadClass("org.mozilla.gecko.GeckoAppShell");
> +            Method disableScreenshotMethod = appshell.getMethod("disableScreenshot");
> +            disableScreenshotMethod.invoke(null);
> +        } catch(ClassNotFoundException ex) {

nit: space between "catch" and "(". Same for all catch blocks.

Comment 6

[Geoff Brown [:gbrown]]

Attachment: (Patch 1) add testCheck3 Talos test

:kats' nits are addressed here.

This adds a new Talos/Robocop test, testCheck3, identical to testCheck2 except that it runs with Fennec's low-res screenshot feature disabled.

Comment 7

[Geoff Brown [:gbrown]]

(In reply to Joel Maher (:jmaher) from comment #1)
> any chance we can get rid of tcheckerboard (original)?  My concern is these
> tests take a long time and eat up our limited resources.

Joel, have your concerns been addressed?

Comment 8

[Joel Maher ( :jmaher ) (UTC -0800)]

Comment on attachment 621747 [details] [diff] [review]
(Patch 1) add testCheck3 Talos test

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

My concerns haven't been addressed.  We are now running 3 different checkerboarding tests for every checkin.  While these work ok, they have a high failure rate and they take a LONG time. 

with that said, we need this test and we should work on getting it turned on this week.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

(In reply to Joel Maher (:jmaher) from comment #8)
> While these work ok, they have a
> high failure rate and they take a LONG time. 
> 

Do we know why (with respect to both failure rate and execution time)? When I run them locally they run pretty fast.

Comment 10

[Joel Maher ( :jmaher ) (UTC -0800)]

failure rate, I am not sure...general tegra problems.

for execution time, we run these tests 5 times in a row.  For each run it requires setup and cleanup which take about 4 minutes total, tracked in bug 736105.  Other than that, the test itself runs pretty fast.

Comment 11

[Joel Maher ( :jmaher ) (UTC -0800)]

Attachment: add talos test definition (1.0)

Comment 12

[Joel Maher ( :jmaher ) (UTC -0800)]

Attachment: add tcheck3 to graph server definitions (1.0)

Comment 13

[Joel Maher ( :jmaher ) (UTC -0800)]

landed graph server definition: http://hg.mozilla.org/graphs/rev/26ed5c8734f9

Comment 14

[Jeff Hammel]

Comment on attachment 621990 [details] [diff] [review]
add talos test definition (1.0)

looks good.  Sorry I took so long to get to this

Comment 15

[Joel Maher ( :jmaher ) (UTC -0800)]

landed the talos bits: http://hg.mozilla.org/build/talos/rev/abd310d24bb5

Comment 16

[Joel Maher ( :jmaher ) (UTC -0800)]

I believe all that is left is to add this test to the releng config.py script.  Can anybody think of anything else?

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Patch 1 landed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e76cfc8fc99

Comment 18

[Ed Morley [:emorley]]

(In reply to Ryan VanderMeulen from comment #17)
> Patch 1 landed.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/3e76cfc8fc99

https://hg.mozilla.org/mozilla-central/rev/3e76cfc8fc99

Comment 19

[Joel Maher ( :jmaher ) (UTC -0800)]

Attachment: [bb-configs] add remote-trobocheck3 as a test to buildbot (1.0)

Comment 20

[Joel Maher ( :jmaher ) (UTC -0800)]

do you want me to land this and file a bug for deployment?

Comment 21

[Armen [:armenzg]]

Yes please.

Comment 22

[Joel Maher ( :jmaher ) (UTC -0800)]

landed buildbot-configs:
http://hg.mozilla.org/build/buildbot-configs/rev/4aa8f14acd08

Comment 23

[Mike Taylor [:bear]]

in production 2012-05-17 0750 PDT

Comment 24

[Joel Maher ( :jmaher ) (UTC -0800)]

Attachment: add tcheck3 to the list of tests (1.0)

apparently we had forgotten to add tcheck3 to the list of tests.  Actually it was an oversight in my part, but nevertheless we are where we are.

Comment 25

[Jeff Hammel]

Comment on attachment 624850 [details] [diff] [review]
add tcheck3 to the list of tests (1.0)

good catch, thanks

Comment 26

[Joel Maher ( :jmaher ) (UTC -0800)]

http://hg.mozilla.org/build/talos/rev/2339d6217ae1

still need to create a new talos.zip and upload it.

Comment 27

[Joel Maher ( :jmaher ) (UTC -0800)]

running on m-c as of this morning:
http://graphs.mozilla.org/graph.html#tests=[[204,11,20]]&sel=none&displayrange=7&datatype=running

Comment 28

[Phil Ringnalda (:philor)]

(In reply to Joel Maher (:jmaher) from comment #10)
> failure rate, I am not sure...general tegra problems.

https://tbpl.mozilla.org/?rev=642d1a36702f is pretty typical. The generalities seem quite specifically in the r* region.

Comment 29

[Phil Ringnalda (:philor)]

So far, since the fixed zip landed, we have 3 pushes on mozilla-central that tried to do something, with 11 failing runs and 2 green runs. On mozilla-inbound, we have 13 pushes that tried to do something, with 32 failures and 5 green runs.

This suite doesn't work.

Comment 30

[Phil Ringnalda (:philor)]

Over the same period on inbound, check2 has 19 failures and 10 green runs.

Comment 31

[Joel Maher ( :jmaher ) (UTC -0800)]

The only changes made to talos were to add a definition for tcheck3 to the config files.  

I have no idea why turning on a new test would do this.  I suspect it has to do with load on the foopies or a hanging bcontroller process.

We can back this out or turn off tcheck3 to see if this fixes the problem.  We had bug 752368, land 3 changesets earlier on m-c, and when it landed it had a lot of red tcheck2 runs.

Comment 32

[Phil Ringnalda (:philor)]

I don't think there's any "did this" - check2 has always been what I thought was awful, 2:1 sounds about normal for it. check3 is just the new definition of awful at 6:1.

Comment 33

[Phil Ringnalda (:philor)]

New favorite: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=ab3f805f3210&jobname=check3