Closed Bug 1233220 Opened 8 years ago Closed 8 years ago

Add an awsy-like test that tracks Firefox for Android memory use in perfherder

Categories

(Firefox for Android Graveyard :: Testing, defect)

defect
Not set
normal

Tracking

(firefox45 fixed, firefox46 fixed, firefox47 fixed)

RESOLVED FIXED
Firefox 47
Tracking Status
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: gbrown, Assigned: gbrown)

Details

Attachments

(1 file, 2 obsolete files)

https://www.areweslimyet.com/mobile/ was great and provided useful memory metrics, but it has been dormant for over a year and I am not aware of any serious plans to revive it, or perhaps more importantly, maintain it.

A possible alternative with some of the same benefits and less maintenance (risk of infrastructure failure) could be achieved by implementing an awsy-like test (start, open tabs, close tabs, track memory at each step) as a regular mochitest or robocop test, running in continuous integration and reporting to perfherder.
I wrote a proof-of-concept test based on https://github.com/staktrace/awsy-armv6, implemented as an android-only mochitest-chrome test. https://treeherder.mozilla.org/#/jobs?repo=try&revision=9628781606d0 successfully reported ("fake" data) to perfherder: https://treeherder.mozilla.org/perf.html#/graphs?series=[try,7b78823aef0468dfd81a557305fc78cf039652b4,1]. Lots to do here still.
I've been talking to Eric Rahm about getting AWSY reporting to treeherder in early 2016. CCing him here.
(In reply to William Lachance (:wlach) from comment #2)
> I've been talking to Eric Rahm about getting AWSY reporting to treeherder in
> early 2016. CCing him here.

I spoke with kats and he has tentatively agreed to revive the tests once we work out how to push the data to treeherder for the desktop version.
(In reply to Eric Rahm [:erahm] from comment #3)
> I spoke with kats and he has tentatively agreed to revive the tests once we
> work out how to push the data to treeherder for the desktop version.

Interesting! I'll press pause on this bug until we see how those efforts proceed.
Here's what I have now. It performs the same operations as awsy-armv6 (start, wait, open tabs, close tabs, force gc, etc) in the form of a regular mochitest-chrome test and reports simple memory stats via PERFHERDER_DATA. See TODO comments for various issues / topics for discussion.

Typical output:

 0:47.86 TEST_START: None mobile/android/tests/browser/chrome/test_memory.html
 0:49.79 LOG: None INFO Start | vsize 707895296 | residentFast 166612992 | heapAllocated 56723304
 1:19.81 LOG: None INFO StartSettled | vsize 708374528 | residentFast 165502976 | heapAllocated 55789708
 2:01.34 LOG: None INFO TabsOpen | vsize 709869568 | residentFast 173248512 | heapAllocated 58940252
 2:31.35 LOG: None INFO TabsOpenSettled | vsize 708624384 | residentFast 172879872 | heapAllocated 58584800
 2:49.77 LOG: None INFO TabsOpenForceGC | vsize 709713920 | residentFast 170016768 | heapAllocated 55257144
 2:50.28 LOG: None INFO TabsClosed | vsize 713965568 | residentFast 170946560 | heapAllocated 58394700
 3:20.29 LOG: None INFO TabsClosedSettled | vsize 713891840 | residentFast 170463232 | heapAllocated 57939832
 3:34.83 LOG: None INFO TabsClosedForceGC | vsize 713891840 | residentFast 169959424 | heapAllocated 57450948
 3:34.84 LOG: None INFO XERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"name": "memory stats Start vsize", "value": 707895296},{"name": "memory stats Start residentFast", "value": 166612992},{"name": "memory stats Start heapAllocated", "value": 56723304},{"name": "memory stats StartSettled vsize", "value": 708374528},{"name": "memory stats StartSettled residentFast", "value": 165502976},{"name": "memory stats StartSettled heapAllocated", "value": 55789708},{"name": "memory stats TabsOpen vsize", "value": 709869568},{"name": "memory stats TabsOpen residentFast", "value": 173248512},{"name": "memory stats TabsOpen heapAllocated", "value": 58940252},{"name": "memory stats TabsOpenSettled vsize", "value": 708624384},{"name": "memory stats TabsOpenSettled residentFast", "value": 172879872},{"name": "memory stats TabsOpenSettled heapAllocated", "value": 58584800},{"name": "memory stats TabsOpenForceGC vsize", "value": 709713920},{"name": "memory stats TabsOpenForceGC residentFast", "value": 170016768},{"name": "memory stats TabsOpenForceGC heapAllocated", "value": 55257144},{"name": "memory stats TabsClosed vsize", "value": 713965568},{"name": "memory stats TabsClosed residentFast", "value": 170946560},{"name": "memory stats TabsClosed heapAllocated", "value": 58394700},{"name": "memory stats TabsClosedSettled vsize", "value": 713891840},{"name": "memory stats TabsClosedSettled residentFast", "value": 170463232},{"name": "memory stats TabsClosedSettled heapAllocated", "value": 57939832},{"name": "memory stats TabsClosedForceGC vsize", "value": 713891840},{"name": "memory stats TabsClosedForceGC residentFast", "value": 169959424},{"name": "memory stats TabsClosedForceGC heapAllocated", "value": 57450948},]}
 3:34.92 TEST_END: None Harness OK. Subtests passed 1/1. Unexpected 0
(In reply to Geoff Brown [:gbrown] from comment #5)
> Created attachment 8701878 [details] [diff] [review]
> work in progress - reports simple memory stats via PERFHERDER_DATA
> 
> Here's what I have now. It performs the same operations as awsy-armv6
> (start, wait, open tabs, close tabs, force gc, etc) in the form of a regular
> mochitest-chrome test and reports simple memory stats via PERFHERDER_DATA.
> See TODO comments for various issues / topics for discussion.
> 

If you can get this running as a mochitest I think it would be superior to the setup I had. I'd rather help you get this finished than keep a device running under my desk like I had before.
running this in automation will be run on emulators- just ensuring that our intention is to run on emulators
Better, but time-outs are common: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b600f82888b6. There are several 30 second delays and GC can take over a minute on the emulator. I think I'll just requestLongerTimeout().
wlach: http://archive.mozilla.org/pub/mobile/try-builds/gbrown@mozilla.com-b600f82888b66832d1340a37431a94d9577867d0/try-android-api-11/try_ubuntu64_vm_armv7_mobile_test-mochitest-chrome-bm68-tests1-linux64-build394.txt.gz contains:

12:34:03     INFO -  222 INFO PERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"name": "test_memory RSS: Fresh start", "value": 203657216},{"name": "test_memory RSS: Fresh start [+30s]", "value": 199626752},{"name": "test_memory RSS: After tabs open", "value": 197287936},{"name": "test_memory RSS: After tabs open [+30s]", "value": 196915200},{"name": "test_memory RSS: After tabs open [+30s, forced GC]", "value": 195252224},{"name": "test_memory RSS: Tabs closed", "value": 203534336},{"name": "test_memory RSS: Tabs closed [+30s]", "value": 202850304},{"name": "test_memory RSS: Tabs closed [+30s, forced GC]", "value": 202575872}]}

If I go to perfherder and try adding data for Project: try / Platform: android-4-3-armv7-api11, I see no options under Tests. Is there something wrong with my PERFHERDER_DATA?
Flags: needinfo?(wlachance)
(In reply to Geoff Brown [:gbrown] from comment #10)
> wlach:
> http://archive.mozilla.org/pub/mobile/try-builds/gbrown@mozilla.com-
> b600f82888b66832d1340a37431a94d9577867d0/try-android-api-11/
> try_ubuntu64_vm_armv7_mobile_test-mochitest-chrome-bm68-tests1-linux64-
> build394.txt.gz contains:
> 
> 12:34:03     INFO -  222 INFO PERFHERDER_DATA: {"framework": {"name":
> "build_metrics"}, "suites": [{"name": "test_memory RSS: Fresh start",
> "value": 203657216},{"name": "test_memory RSS: Fresh start [+30s]", "value":
> 199626752},{"name": "test_memory RSS: After tabs open", "value":
> 197287936},{"name": "test_memory RSS: After tabs open [+30s]", "value":
> 196915200},{"name": "test_memory RSS: After tabs open [+30s, forced GC]",
> "value": 195252224},{"name": "test_memory RSS: Tabs closed", "value":
> 203534336},{"name": "test_memory RSS: Tabs closed [+30s]", "value":
> 202850304},{"name": "test_memory RSS: Tabs closed [+30s, forced GC]",
> "value": 202575872}]}
> 
> If I go to perfherder and try adding data for Project: try / Platform:
> android-4-3-armv7-api11, I see no options under Tests. Is there something
> wrong with my PERFHERDER_DATA?

Yes. :) It doesn't conform to the perfherder schema (https://github.com/mozilla/treeherder/blob/master/schemas/performance-artifact.json), which requires a subtests parameter.

I'd do several things here:

1. Validate your output against the perfherder schema when you're testing, this will make errors easier to spot. We already do this for talos, you can see an example here: http://mxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/talos.py#326
2. Restructure this so that you have one suite and several subtests. let's call the suite "test_memory". Then you can remove the "test_memory RSS:" from each name, ending up with something like this:
{
    "framework": {
        "name": "build_metrics"
    },
    "suites": [
        {
            "name": "test_memory RSS",
            "subtests": [
                {
                    "name": "Fresh start [+30s]",
                    "value": 199626752
                },
                {
                    "name": "After tabs open",
                    "value": 197287936
                },
                {
                    "name": "After tabs open [+30s]",
                    "value": 196915200
                },
                {
                    "name": "After tabs open [+30s, forced GC]",
                    "value": 195252224
                },
                {
                    "name": "Tabs closed",
                    "value": 203534336
                },
                {
                    "name": "Tabs closed [+30s]",
                    "value": 202850304
                },
                {
                    "name": "Tabs closed [+30s, forced GC]",
                    "value": 202575872
                }
            ]
        }
    ]
}

Also, using "build_metrics" as a test framework doesn't really make sense longer term. Maybe we could reuse AWSY, which we'll be adding for :erahm?
Flags: needinfo?(wlachance)
An update, not quite ready for review, but feedback welcome.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=574a1a201ee6 has, for example:

10:41:05     INFO -  213 INFO TEST-START | mobile/android/tests/browser/chrome/test_memory.html
10:41:05     INFO -  214 INFO Fresh start | RSS: 210538496
10:41:38     INFO -  215 INFO Fresh start [+30s] | RSS: 206839808
10:42:23     INFO -  216 INFO After tabs | RSS: 199753728
10:42:44     INFO -  217 INFO After tabs [+30s] | RSS: 199774208
10:44:00     INFO -  218 INFO After tabs [+30s, forced GC] | RSS: 198635520
10:44:00     INFO -  219 INFO Tabs closed | RSS: 213405696
10:44:32     INFO -  220 INFO Tabs closed [+30s] | RSS: 209489920
10:45:38     INFO -  221 INFO Tabs closed [+30s, forced GC] | RSS: 209125376
10:45:38     INFO -  222 INFO PERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"name": "test_memory RSS", "subtests": [{"name": "Fresh start", "value": 210538496}, {"name": "Fresh start [+30s]", "value": 206839808}, {"name": "After tabs", "value": 199753728}, {"name": "After tabs [+30s]", "value": 199774208}, {"name": "After tabs [+30s, forced GC]", "value": 198635520}, {"name": "Tabs closed", "value": 213610496}, {"name": "Tabs closed [+30s]", "value": 209489920}, {"name": "Tabs closed [+30s, forced GC]", "value": 209125376}]}]}
10:45:38     INFO -  223 INFO TEST-PASS | mobile/android/tests/browser/chrome/test_memory.html | memory logging complete -- see results in Perfherder
10:45:38     INFO -  224 INFO TEST-OK | mobile/android/tests/browser/chrome/test_memory.html | took 274846ms

and the first results can be seen in Perfherder now:

https://treeherder.mozilla.org/perf.html#/graphs?series=[try,2b5a87368f3600ea773df619dc5c3a6d6fcc6de7,1]&series=[try,0e7e7dc2a24aeb5dc4a6e9ba9329dc05e00c6554,1]&series=[try,89cc7b65982a1c2ea46d1c136a6a51fca87cd53c,1]&series=[try,314f725b0879a1c6698f9ce27d20e31f6a3e5e52,1]&series=[try,6743e47d13298b72dc933917fc548daa6c65fbe2,1]&series=[try,5b7a2ce9171ac84ae2abad156bafcb98b4825b7e,1]&series=[try,c29e1bb8540af8e4d0fe8066ea61787c5beea3e1,1]&series=[try,33c54f677570fa35be45d3f751af10d1e16bf585,1]

Outstanding issues:
 - I am using some dummy pages to be opened in tabs; we may be able to use tp5 pages -- there is a discussion happening in a legal bug.
 - We should use a framework name of "AWSY" instead of "build_metrics", but "AWSY" cannot be used just yet.
 - I should validate against the schema in the test.
Attachment #8701878 - Attachment is obsolete: true
Attachment #8706528 - Flags: feedback?(snorp)
BTW, gc takes so long on debug that the test times out. I'm skipping the test on debug builds. Is that going to be okay?
(In reply to Geoff Brown [:gbrown] from comment #13)
> BTW, gc takes so long on debug that the test times out. I'm skipping the
> test on debug builds. Is that going to be okay?

Do we care about this information on debug builds? We don't normally run talos against that class of builds. I think my preference would be to only run this on opt/pgo.
(In reply to William Lachance (:wlach) from comment #11)
> (In reply to Geoff Brown [:gbrown] from comment #10)
> 2. Restructure this so that you have one suite and several subtests. let's
> call the suite "test_memory". Then you can remove the "test_memory RSS:"
> from each name, ending up with something like this:
> {
>     "framework": {
>         "name": "build_metrics"
>     },
>     "suites": [
>         {
>             "name": "test_memory RSS",
>             "subtests": [
>                 {
>                     "name": "Fresh start [+30s]",
>                     "value": 199626752
>                 },
>                 {
>                     "name": "After tabs open",
>                     "value": 197287936
>                 },
>                 {
>                     "name": "After tabs open [+30s]",
>                     "value": 196915200
>                 },
>                 {
>                     "name": "After tabs open [+30s, forced GC]",
>                     "value": 195252224
>                 },
>                 {
>                     "name": "Tabs closed",
>                     "value": 203534336
>                 },
>                 {
>                     "name": "Tabs closed [+30s]",
>                     "value": 202850304
>                 },
>                 {
>                     "name": "Tabs closed [+30s, forced GC]",
>                     "value": 202575872
>                 }
>             ]
>         }
>     ]
> }
> 
> Also, using "build_metrics" as a test framework doesn't really make sense
> longer term. Maybe we could reuse AWSY, which we'll be adding for :erahm?

Yes that makes sense, we landed it as 'awsy'.

It would definitely be nice to have this data line up w/ the desktop data. The proposed subtest names look good to me (and line up w/ what we see on areweslimyet.com).

For suites I was planning the following:
 - "Resident Memory" (8 data points)
 - "Explicit Memory" (8 data points)
 - "Heap Unclassified"
   - "Heap Unclassified: After tabs [+30s]"
 - "JS"
   - "JS: After tabs [+30s]"
 - "Images"
   - "Images: After tabs [+30s]"

We can bikeshed those if you'd like, I just want to make sure we use the same names in the end. Additionally we could do the full 8 datapoints for the last 3 suites, I believe we currently don't do that on the AWSY website purely for aesthetic reasons (too many graphs).

jmaher and wlach suggested that we report a geometric mean of the subtest values with each suite as well. ie:

>    "suites": [
>        {
>            "name": "Resident Memory",
>            "subtests": [ ... ],
>            "value": geometric_mean(subtests.values)
>        }
(In reply to William Lachance (:wlach) from comment #14)
> (In reply to Geoff Brown [:gbrown] from comment #13)
> > BTW, gc takes so long on debug that the test times out. I'm skipping the
> > test on debug builds. Is that going to be okay?
> 
> Do we care about this information on debug builds? We don't normally run
> talos against that class of builds. I think my preference would be to only
> run this on opt/pgo.

For desktop we certainly only run the tests against opt (and I believe historically the same was true for mobile).
Comment on attachment 8706528 [details] [diff] [review]
report simple memory stats via PERFHERDER_DATA

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

::: mobile/android/tests/browser/chrome/test_memory.html
@@ +20,5 @@
> +  "use strict";
> +
> +  const { interfaces: Ci, utils: Cu, classes: Cc } = Components;
> +
> +  // TODO: decide on test pages. tp5?

Yeah, with these test pages being so simple, I'm not sure how useful the test is going to be. Gotta start somewhere I guess.

@@ +42,5 @@
> +  SimpleTest.requestCompleteLog();  // so that "PERFHERDER_DATA" can be scraped from the log
> +
> +  function logMemory(aLabel) {
> +    var mrm = Cc["@mozilla.org/memory-reporter-manager;1"].getService(Ci.nsIMemoryReporterManager);
> +    info(aLabel + " | RSS: " + mrm.resident);

This and other string building is probably easier/clearer if you use the es6 string templating (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/template_strings)

info(`${aLabel} | RSS: ${mrm.resident}`);

@@ +202,5 @@
> +  }
> +
> +  function postClosing() {
> +    logMemory("Tabs closed [+30s]");
> +    doFullGc(function() {

Can you use a fat arrow function. Instead of function(){}.bind(this) you can do () => {}
Attachment #8706528 - Flags: feedback?(snorp) → feedback+
Updated for feedback from :snorp and :erahm - thanks! 

I am still using dummy pages to be opened in tabs; let's discuss real data in a separate patch.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8c2fc516f7c includes:

http://archive.mozilla.org/pub/mobile/try-builds/gbrown@mozilla.com-d8c2fc516f7cf60bc65a3e4a1da3ac3e180d894f/try-android-api-9/try_ubuntu64_vm_mobile_test-mochitest-chrome-bm115-tests1-linux64-build522.txt.gz which has:

10:58:11     INFO -  224 INFO PERFHERDER_DATA: {"framework": {"name": "build_metrics"}, "suites": [{"name": "Resident Memory", "subtests": [{"name": "Fresh start", "value": 215166976}, {"name": "Fresh start [+30s]", "value": 209915904}, {"name": "After tabs", "value": 200400896}, {"name": "After tabs [+30s]", "value": 199815168}, {"name": "After tabs [+30s, forced GC]", "value": 199815168}, {"name": "Tabs closed", "value": 214589440}, {"name": "Tabs closed [+30s]", "value": 210210816}, {"name": "Tabs closed [+30s, forced GC]", "value": 210210816}], "value": 207425430}]}

which can be viewed at:

https://treeherder.mozilla.org/perf.html#/graphs?series=[try,70b95e1b3a0377354a7674cf8412a83edaf731db,1]&series=[try,a11958519f72606dfe2c1665dea77ee454b921bc,1]&series=[try,d06c7a6886e32820aace062a8f0131ea20b2b332,1]&series=[try,879f9eecd1e35dc7885cf88de0eceeaed074c1b7,1]&series=[try,6872ac99dcb05baa54328343c9590f65ef78d1a0,1]&series=[try,661147c0d9101c971bd74533880aa825f25d8e23,1]&series=[try,92f0f9fa5d8e0e2bfb4ffa44d061b3f6ace51b19,1]&series=[try,e4fb1f5ac50c962ee457fa0bc0a139555565e864,1]&series=[try,f7ae9e1f98ad4b17da45f52cbdd05b773a358463,1]

(this try run still uses the "build_metrics" framework name -- changed to "awsy" in the final patch).

As before, most of this patch is made up of code from https://github.com/staktrace/awsy-armv6/blob/master/fennec-addon-awsy/bootstrap.js, with minor adaptations for the mochitest environment. Memory measurements are made at the same 8 points in the Firefox "life-cycle" as on areweslimyet.com. For now, only "Resident Memory" is recorded and reported; additional measurements should be easy to add.
Attachment #8706528 - Attachment is obsolete: true
Attachment #8707910 - Flags: review?(snorp)
So, how many pages and which pages should we open? 

As I understand it, desktop awsy opens 100 pages from tp5, but ensures that no more than 30 tabs are open at one time. areweslimyet.com/mobile used 15 pages from tp5:

    thesartorialist.blogspot.com/thesartorialist.blogspot.com/index.html
    cakewrecks.blogspot.com/cakewrecks.blogspot.com/index.html
    baidu.com/www.baidu.com/s@wd=mozilla.html
    twitter.com/twitter.com/ICHCheezburger.html
    msn.com/www.msn.com/index.html
    yahoo.co.jp/www.yahoo.co.jp/index.html
    amazon.com/www.amazon.com/Kindle-Wireless-Reader-Wifi-Graphite/dp/B002Y27P3M/507846.html
    linkedin.com/www.linkedin.com/in/christopherblizzard@goback=.nppvan_%252Flemuelf.html
    bing.com/www.bing.com/search@q=mozilla&go=&form=QBLH&qs=n&sk=&sc=8-0.html
    icanhascheezburger.com/icanhascheezburger.com/index.html
    yandex.ru/yandex.ru/yandsearch@text=mozilla&lr=21215.html
    cgi.ebay.com/cgi.ebay.com/ALL-NEW-KINDLE-3-eBOOK-WIRELESS-READING-DEVICE-W-WIFI-/130496077314@pt=LH_DefaultDomain_0&hash=item1e622c1e02.html
    163.com/www.163.com/index.html
    mail.ru/mail.ru/index.html
    bbc.co.uk/www.bbc.co.uk/news/index.html

https://treeherder.mozilla.org/#/jobs?repo=try&revision=29e3e3d84972 tries using just the first 5 pages:
    thesartorialist.blogspot.com/thesartorialist.blogspot.com/index.html
    cakewrecks.blogspot.com/cakewrecks.blogspot.com/index.html
    baidu.com/www.baidu.com/s@wd=mozilla.html
    twitter.com/twitter.com/ICHCheezburger.html
    msn.com/www.msn.com/index.html

That produced:
  200 INFO Fresh start | Resident Memory: 195338240
  201 INFO Fresh start [+30s] | Resident Memory: 193650688
  202 INFO After tabs | Resident Memory: 248963072
  203 INFO After tabs [+30s] | Resident Memory: 251740160
  204 INFO After tabs [+30s, forced GC] | Resident Memory: 251768832
  205 INFO Tabs closed | Resident Memory: 232620032
  206 INFO Tabs closed [+30s] | Resident Memory: 210501632
  207 INFO Tabs closed [+30s, forced GC] | Resident Memory: 210501632

which looks much better to me than using my dummy pages -- you see a significant increase in RSS after tabs are opened now.

Test run time with these 5 tp5 pages increased to about 6.5 minutes (compared to 3 minutes with dummy pages).

The size of the mochitest test zip increased to about 67 MB (compared to 57 MB with dummy pages).

Those 5 tp5 pages account for about 13 MB (uncompressed) in mozilla-central.


I feel like these 5 pages are about "right": Big enough to observe the effect on RSS with acceptable cost to run time/test zip size/mozilla-central size. I am hesitant to check in anything bigger. Your thoughts?
Flags: needinfo?(snorp)
Flags: needinfo?(jmaher)
5 pages sound good.  I think we need to start somewhere and this gives us data showing an increase.  Maybe after a couple months we can revisit to determine if we are getting regressions, need more pages, etc.
Flags: needinfo?(jmaher)
(In reply to Joel Maher (:jmaher) from comment #20)
> 5 pages sound good.  I think we need to start somewhere and this gives us
> data showing an increase.  Maybe after a couple months we can revisit to
> determine if we are getting regressions, need more pages, etc.

Totally agree.

Great work!
Comment on attachment 8707910 [details] [diff] [review]
report simple memory stats via PERFHERDER_DATA

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

lgtm, but we should have a front-end person review too.
Attachment #8707910 - Flags: review?(snorp)
Attachment #8707910 - Flags: review?(margaret.leibovic)
Attachment #8707910 - Flags: review+
Comment on attachment 8707910 [details] [diff] [review]
report simple memory stats via PERFHERDER_DATA

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

I just gave this a quick glance, but generally seems fine to me. Land it! We can always make it better as we need to.
Attachment #8707910 - Flags: review?(margaret.leibovic) → review+
Also, could we uplift this to start seeing data for other trees? I'd be interested to see regressions between versions.
(In reply to :Margaret Leibovic from comment #24)
> Also, could we uplift this to start seeing data for other trees? I'd be
> interested to see regressions between versions.

Good idea - will do.
The patch for swapping dummy pages for tp5 pages is too big to attach, but was r+'d by snorp and margaret via irc.
Flags: needinfo?(snorp)
Still no check-in here, 'cause I'm waiting for confirmation from Legal that it is okay to land tp5-derived material in mozilla-central.
I'm changing 2 of the 5 pages selected in Comment 19, following discussion in bug 1239882. The new page set is:

    amazon.com/www.amazon.com/Kindle-Wireless-Reader-Wifi-Graphite/dp/B002Y27P3M/507846.html
    cgi.ebay.com/cgi.ebay.com/ALL-NEW-KINDLE-3-eBOOK-WIRELESS-READING-DEVICE-W-WIFI-/130496077314@pt=LH_DefaultDomain_0&hash=item1e622c1e02.html
    baidu.com/www.baidu.com/s@wd=mozilla.html
    twitter.com/twitter.com/ICHCheezburger.html
    msn.com/www.msn.com/index.html

All are still selections from the original pages used in areweslimyet.com/mobile.

New try runs:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=47ea1d77ccb2
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7379ead3c8ac
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3e2af54c2fd

https://treeherder.mozilla.org/perf.html#/graphs?timerange=604800&series=[try,7444fa93137385123557a385c81efc99ed82546c,1]&series=[try,d06c7a6886e32820aace062a8f0131ea20b2b332,0]&series=[try,6872ac99dcb05baa54328343c9590f65ef78d1a0,0]&series=[try,59908c3d619c02498493e97926f4c94468dd0fca,1]&series=[try,7328297dec0adfb1fb1be9905308e5d3d89c3d32,1]

is promising: minimal noise, the start of a good graph!
I had to back these out because they apparently broke spidermonkey builds (but apparently only the Windows SM builds):
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c91719ddb14

https://treeherder.mozilla.org/logviewer.html#?job_id=20504441&repo=mozilla-inbound
Flags: needinfo?(gbrown)
Thanks.

It looks like some of the data files have filenames that are not valid on Windows. I'll rename and verify Windows builds on try.
Flags: needinfo?(gbrown)
 IOError: [Errno 2] c:\builds\moz2_slave\m-in_w32-d_sm-compacting-00000\src\mobile/android/tests/browser/chrome/tp5/amazon.com/www.amazon.com/Kindle-Wireless-Reader-Wifi-Graphite/dp/B002Y27P3M/%5C%22http%3A/g-ecx.images-amazon.com/images/G/01/kindle/shasta/photos/icon_newwindow._V209801570_.jpg%5C%22.html: The system cannot find the path specified
 abort: c:\builds\moz2_slave\m-in_w32-d_sm-compacting-00000\src\mobile/android/tests/browser/chrome/tp5/amazon.com/www.amazon.com/Kindle-Wireless-Reader-Wifi-Graphite/dp/B002Y27P3M/%5C%22http%3A/g-ecx.images-amazon.com/images/G/01/kindle/shasta/photos/icon_newwindow._V209801570_.jpg%5C%22.html: The system cannot find the path specified
 command: ERROR

 Traceback (most recent call last):

   File "c:/builds/moz2_slave/m-in_w32-d_sm-compacting-00000/scripts/buildfarm/utils\../../lib/python\util\commands.py", line 52, in run_cmd

     return subprocess.check_call(cmd, **kwargs)

   File "c:\mozilla-build\python27\lib\subprocess.py", line 542, in check_call

     raise CalledProcessError(retcode, cmd)

 CalledProcessError: Command '['hg', 'update', '-C', '-r', 'f920df4d107f5e4933c5e4025e1033aa0d1617b0']' returned non-zero exit status 255
Also http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-inbound-win64/1453829369/mozilla-inbound-win64-bm73-build1-build588.txt.gz

09:33:48     INFO -  WindowsError: [Error 206] The filename or extension is too long: 'c:\\builds\\moz2_slave\\m-in-w64-000000000000000000000\\build\\src\\mobile/android/tests/browser/chrome/tp5/amazon.com/www.amazon.com/Kindle-Wireless-Reader-Wifi-Graphite/dp/B002Y27P3M/%5C%22http%3A/g-ecx.images-amazon.com/images/G/01/kindle/shasta/photos'
09:33:48    ERROR -  abort: The filename or extension is too long: 'c:\builds\moz2_slave\m-in-w64-000000000000000000000\build\src\mobile/android/tests/browser/chrome/tp5/amazon.com/www.amazon.com/Kindle-Wireless-Reader-Wifi-Graphite/dp/B002Y27P3M/%5C%22http%3A/g-ecx.images-amazon.com/images/G/01/kindle/shasta/photos'
Carefully choosing some different tp5 pages - again! - keeps paths from getting too long for Windows:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3456be80d888
https://hg.mozilla.org/mozilla-central/rev/3037141dcdcf
https://hg.mozilla.org/mozilla-central/rev/d00a30ae268e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Hum, are we really adding TP5 data to the tree? I thought part of the reason the TP5 data was a) out-of-tree b) non-public was because copyright applies to those files. Gerv, thoughts?
Flags: needinfo?(gerv)
I consulted Legal in bug 1239882.
If it's been signed off by Legal, I have no problem with it. Can we please add a README in the directory saying:

"This directory contains pages and other resources downloaded from the web for the purpose of testing against pages from the real world. They are not made available under an open source license."

We don't have to state what license they _are_ available under. :-)

Gerv
Flags: needinfo?(gerv)
Shouldn't we move it under, say, other-licenses, then, to make it easier to find/notice for people who care about it? I do care (with my Debian hat on), and I only found out because Kairo mentioned that the changeset in question was breaking hgweb's pushlog's html.
Flags: needinfo?(gerv)
Or really, why do we have to add non open-source bits in the tree?
(In reply to Mike Hommey [:glandium] from comment #42)
> Or really, why do we have to add non open-source bits in the tree?

Not a question for me :-) Although I kind of agree it would be preferable out of the main tree. I don't supposed we can wget it before the tests start, or something?

As for putting it under other-licenses, it seems like that was an experiment which didn't work - there's non-MPLed code all over the tree. I guess non-open-source is a bit different, so we could perhaps repurpose it for that.

Gerv
Flags: needinfo?(gerv)
(In reply to Mike Hommey [:glandium] from comment #42)
> Or really, why do we have to add non open-source bits in the tree?

I won't say it's a necessity, but tp5 data is preferred for performance tests because it is (somewhat) representative of data encountered by browsers in real life. Talos and awsy use tp5 and I want this test to have some consistency with that tradition. Of course, mochitests normally check in data files alongside the test. I can't think of any existing mochitest mechanism for getting data from another source. I think some new mechanism could be added to the mochitest harness but we'd need to be mindful of the https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy wrt accessing external data sources.
we could grab tp5n.zip from tooltool as we do for talos (when we fix the windows issue).  mochitests install packages and tools to bootstrap with mozharness.  This then becomes questionable for developers with |mach test ...|, that needs to download the data and store it.  We have a patch that works for talos to help out with this.  I am not sure what other factors would need to be considered.
:gerv -- Thanks for the README wording suggestion; that's added now. ^^
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.