Closed Bug 751158 Opened 10 years ago Closed 10 years ago

Create tcheckboard3 to measure checkerboard with low res off

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 15

People

(Reporter: jpr, Assigned: jmaher)

References

Details

Attachments

(5 files, 1 obsolete file)

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.
Assignee: nobody → gbrown
any chance we can get rid of tcheckerboard (original)?  My concern is these tests take a long time and eat up our limited resources.
(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.
(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).
Attached patch a preliminary attempt (obsolete) — Splinter Review
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?
Attachment #621189 - Flags: feedback?(bugmail.mozilla)
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.
Attachment #621189 - Flags: feedback?(bugmail.mozilla) → feedback+
: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.
Attachment #621189 - Attachment is obsolete: true
Attachment #621747 - Flags: review?(jmaher)
(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 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.
Attachment #621747 - Flags: review?(jmaher) → review+
(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.
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.
Attachment #621990 - Flags: review?(jhammel)
Attachment #621991 - Flags: review?(armenzg) → review+
landed graph server definition: http://hg.mozilla.org/graphs/rev/26ed5c8734f9
Depends on: 753767
Comment on attachment 621990 [details] [diff] [review]
add talos test definition (1.0)

looks good.  Sorry I took so long to get to this
Attachment #621990 - Flags: review?(jhammel) → review+
I believe all that is left is to add this test to the releng config.py script.  Can anybody think of anything else?
Attachment #621747 - Attachment description: add testCheck3 Talos test → (Patch 1) add testCheck3 Talos test
Keywords: checkin-needed
Whiteboard: (patch 1 needs to land on m-c)
Patch 1 landed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e76cfc8fc99
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: (patch 1 needs to land on m-c)
Target Milestone: --- → Firefox 15
Attachment #624418 - Flags: review?(armenzg) → review+
do you want me to land this and file a bug for deployment?
Yes please.
Depends on: 756071
in production 2012-05-17 0750 PDT
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.
Assignee: gbrown → jmaher
Status: NEW → ASSIGNED
Attachment #624850 - Flags: review?(jhammel)
Comment on attachment 624850 [details] [diff] [review]
add tcheck3 to the list of tests (1.0)

good catch, thanks
Attachment #624850 - Flags: review?(jhammel) → review+
http://hg.mozilla.org/build/talos/rev/2339d6217ae1

still need to create a new talos.zip and upload it.
Depends on: 756346
running on m-c as of this morning:
http://graphs.mozilla.org/graph.html#tests=[[204,11,20]]&sel=none&displayrange=7&datatype=running
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(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.
Depends on: 756721
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.
Over the same period on inbound, check2 has 19 failures and 10 green runs.
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.
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.
Depends on: 756817
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.