ABICT, PerfConfigurator doesn't display anywhere or have an option to display the available --activeTests. It should. http://hg.mozilla.org/build/talos/file/eeee5b50266b/talos/PerfConfigurator.py#l93

Also, using python PerfConfigurator.py -a anonexistenttest -e `which firefox` ... silently succeeds! But leaves you with a .yml file with no tests :( Really, we should have a definition of what a test is and what tests we have available

This patch moves test definitions to their own file, test.py, and reads as from canon from there. The remaining `tests:` section in the .config files are for overrides to the canonical test definitions, as do several variables from PerfConfigurator. A --print-tests switch is added to print the tests as computed or to print the entire tests available if none are specified. Ultimately the goal is to keep the configuration seeds out of .config files which are duplicated multiply. While this is an interim solution, IMHO it is better than what we have now and a good path forward. Nothing is done with the basetest section which functions identically to how it was done previously. An ultimate solution should do something with this. Careful testing reveals that using the relevent tests plus config files *should* result in the same output configuration. I've tested a bit locally but will push to try.

Comment on attachment 617702 [details] [diff] [review] one small step for jhammel... Review of attachment 617702 [details] [diff] [review]: ----------------------------------------------------------------- In general I like this, I need to give a second pass on the *PerfConfigurator.py files . ::: talos/cycles.config @@ +109,5 @@ > tprender: False > tpdelay: > > tests : > +- name: tdhtml why are tests still in here? ::: talos/remote.config @@ +119,5 @@ > tprender: False > tpdelay: 1000 > > tests : > - name: ts why are tests in here? ::: talos/remotePerfConfigurator.py @@ +87,5 @@ > > + url = pc.PerfConfigurator.convertUrlToRemote(self, url) > + if 'winopen.xul' in url: > + self.buildRemoteTwinopen() > + newline = 'file://' + self.deviceroot + '/talos/' + part did you test this on mobile stuff? I suspect we could have >1 instance in the url line that needs replacing. ::: talos/sample.2.config @@ +115,1 @@ > - name: tp5r moar tests in this file as well. ::: talos/test.py @@ +167,5 @@ > + > +class tgfx(PageloaderTest): > + tpmanifest = '${talos}/page_load_test/gfx/gfx.manifest' > + tpcycles = 5 > + tprender = True we don't run this anymore ::: talos/xperf.config @@ +113,5 @@ > tprender: False > tpdelay: > > tests : > - name: ts_paint a lot of tests in this file

Try run for 7813bb405ea0 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=7813bb405ea0 Results (out of 76 total builds): exception: 3 success: 52 failure: 21 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla.com-7813bb405ea0

all android tests fail, it appears to be related to: Traceback (most recent call last): File "remotePerfConfigurator.py", line 184, in <module> sys.exit(main()) File "remotePerfConfigurator.py", line 170, in main configurator.writeConfigFile() File "/builds/tegra-250/talos-data/talos/PerfConfigurator.py", line 204, in writeConfigFile newline = self.convertLine(line) File "remotePerfConfigurator.py", line 41, in convertLine printMe, newline = pc.PerfConfigurator.convertLine(self, line, testMode, printMe) NameError: global name 'testMode' is not defined All the tp5row tests failed: NOISE: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init]" nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"

(In reply to Joel Maher (:jmaher) from comment #5) > all android tests fail, it appears to be related to: > Traceback (most recent call last): > File "remotePerfConfigurator.py", line 184, in <module> > sys.exit(main()) > File "remotePerfConfigurator.py", line 170, in main > configurator.writeConfigFile() > File "/builds/tegra-250/talos-data/talos/PerfConfigurator.py", line 204, > in writeConfigFile > newline = self.convertLine(line) > File "remotePerfConfigurator.py", line 41, in convertLine > printMe, newline = pc.PerfConfigurator.convertLine(self, line, testMode, > printMe) > NameError: global name 'testMode' is not defined I fixed this at least. New patch posted > All the tp5row tests failed: > NOISE: [Exception... "Component returned failure code: 0x80520012 > (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init]" nsresult: "0x80520012 > (NS_ERROR_FILE_NOT_FOUND)" Will look at this closer in a bit

The reason there are still tests in the .config files is that the parameters for the tests are often changed across files. To give one deliberate example, cycles.config has near identical tests to sample.config but tpcycles is greatly increased to 100 for several tests. I can't tell, in general, if the divergences are intentional or not. Using tests.py for test definitions and .config files for overrides (together with basetest and command line switches) should give identical resulting configurations without having to change how the tests are invoked, which I have been working under since the alternative is to block on buildbot reconfiguration. I would in general like to get as much out of the .config files as possible but it is a long, slow process. See bug 704654 . I'm not sure what tests we are content letting go, etc. There seems to be a lot of duplication

> ::: talos/remotePerfConfigurator.py > @@ +87,5 @@ > > > > + url = pc.PerfConfigurator.convertUrlToRemote(self, url) > > + if 'winopen.xul' in url: > > + self.buildRemoteTwinopen() > > + newline = 'file://' + self.deviceroot + '/talos/' + part > > did you test this on mobile stuff? I suspect we could have >1 instance in the url line that needs replacing. I haven't tested with twinopen yet, but I will do so. The code here should be a straight port of http://hg.mozilla.org/build/talos/file/61d1a38b5c75/talos/remotePerfConfigurator.py#l91 ABICT, the 'parts' is strictly for the case of 'url: http://example.com/foo?bar=My%20Name \n'.split(' ') -> ['url:', '', 'http://...', '\n'] as the urls have no whitespace in them. Now, url is more programmatic so the methods had to change a bit so i just cleaned up here (though i missed 'part' -> 'url', will upload a patch soon). OTOH, with the existing system, it would be better to do this a little better anyway.

Comment on attachment 617898 [details] [diff] [review] kill tgfx and other fixes Review of attachment 617898 [details] [diff] [review]: ----------------------------------------------------------------- ::: talos/PerfConfigurator.py @@ +98,5 @@ > # Several tokens, such as `extensions: []`, `filters: []` > # exist and are required to exist in the source .config files > # for the sake of parsing. We are also very whitespace sensitive. > # https://bugzilla.mozilla.org/show_bug.cgi?id=725097 > # should fix this. is this comment still valid? @@ +250,5 @@ > + # directly translateable keys: > + for key in ('responsiveness', 'tpcycles', 'tppagecycles', 'rss', 'tpdelay'): > + value = getattr(self, key) > + if value: > + test_overrides[key] = value this is confusing at first. we are overriding the override! @@ +531,5 @@ > > + self.add_option('--print-tests', dest="print_tests", > + action='store_true', default=False, > + help="Print the resulting tests configuration to stdout (or print available tests if --activeTests not specified)") > + all other cli options use a separate line for defaults, i.e.: defaults["print_tests"] = False ::: talos/cycles.config @@ +109,5 @@ > tprender: False > tpdelay: > > tests : > +- name: tdhtml we can get rid of cycles.config now, it was just used for the temporary branch before the days of talos.json ::: talos/xperf.config @@ +114,5 @@ > tpdelay: > > tests : > - name: ts_paint > + cycles: 1 put cycles:1 as a basetest variable as well. @@ +119,2 @@ > xperf_providers : ['PROC_THREAD', 'LOADER', 'HARD_FAULTS', 'FILENAME', 'FILE_IO', 'FILE_IO_INIT'] > xperf_stackwalk : ['FileRead', 'FileWrite', 'FileFlush'] I would rather put xperf_providers and xperf_stackwalk as basetest variables.

(In reply to Joel Maher (:jmaher) from comment #11) > Comment on attachment 617898 [details] [diff] [review] > kill tgfx and other fixes > > Review of attachment 617898 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: talos/PerfConfigurator.py > @@ +98,5 @@ > > # Several tokens, such as `extensions: []`, `filters: []` > > # exist and are required to exist in the source .config files > > # for the sake of parsing. We are also very whitespace sensitive. > > # https://bugzilla.mozilla.org/show_bug.cgi?id=725097 > > # should fix this. > > is this comment still valid? Yes, sadly :/ > @@ +250,5 @@ > > + # directly translateable keys: > > + for key in ('responsiveness', 'tpcycles', 'tppagecycles', 'rss', 'tpdelay'): > > + value = getattr(self, key) > > + if value: > > + test_overrides[key] = value > > this is confusing at first. we are overriding the override! Yeah, the order is (test.py) -> foo.config -> command line options To *make* a .yml file, and then additional complexity on the run_tests.py side. o_O It is very convoluted at the moment, but hopefully will get better. > @@ +531,5 @@ > > > > + self.add_option('--print-tests', dest="print_tests", > > + action='store_true', default=False, > > + help="Print the resulting tests configuration to stdout (or print available tests if --activeTests not specified)") > > + > > all other cli options use a separate line for defaults, i.e.: > defaults["print_tests"] = False Fixed. > ::: talos/cycles.config > @@ +109,5 @@ > > tprender: False > > tpdelay: > > > > tests : > > +- name: tdhtml > > we can get rid of cycles.config now, it was just used for the temporary > branch before the days of talos.json Cool, done.

> ::: talos/xperf.config > @@ +114,5 @@ > > tpdelay: > > > > tests : > > - name: ts_paint > > + cycles: 1 > > put cycles:1 as a basetest variable as well. > > @@ +119,2 @@ > > xperf_providers : ['PROC_THREAD', 'LOADER', 'HARD_FAULTS', 'FILENAME', 'FILE_IO', 'FILE_IO_INIT'] > > xperf_stackwalk : ['FileRead', 'FileWrite', 'FileFlush'] > > I would rather put xperf_providers and xperf_stackwalk as basetest variables. Great ideas! Done

carrying r+ forward. this is likely bitrotted so that will have to be fixed

Hmm, still getting the latter error on try: NOISE: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init]" nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)" location: "JS frame :: chrome://pageloader/content/pageloader.js :: plLoadURLsFromURI :: line 647" data: no] NOISE: NOISE: __FAILbrowser frozen__FAIL NOISE: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init]" nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)" location: "JS frame :: chrome://pageloader/content/pageloader.js :: plLoadURLsFromURI :: line 647" data: no] NOISE: NOISE: __FAILbrowser frozen__FAIL NOISE: Found crashdump: /tmp/tmpnaRhIo/profile/minidumps/0b08b020-0988-e352-32970a14-5bcb42e7.dmp I don't see this locally :(

The one on XP is different: NOISE: MOZ_EVENT_TRACE sample 1335297171640 1251 Traceback (most recent call last): File "c:\talos-slave\talos-data\talos\bcontroller.py", line 252, in ? sys.exit(main()) File "c:\talos-slave\talos-data\talos\bcontroller.py", line 249, in main bcontroller.run() File "c:\talos-slave\talos-data\talos\bcontroller.py", line 192, in run results_file = open(self.browser_log, "a") IOError: [Errno 13] Permission denied: 'browser_output.txt' Failed tp5row: Not sure if that is my fault

on trobopan: Traceback (most recent call last): File "remotePerfConfigurator.py", line 183, in <module> sys.exit(main()) File "remotePerfConfigurator.py", line 169, in main configurator.writeConfigFile() File "/builds/tegra-249/talos-data/talos/PerfConfigurator.py", line 210, in writeConfigFile newline = self.convertLine(line) File "remotePerfConfigurator.py", line 41, in convertLine newline = pc.PerfConfigurator.convertLine(self, line) File "/builds/tegra-249/talos-data/talos/PerfConfigurator.py", line 169, in convertLine newline = 'init_url: %s' % self.convertUrlToRemote(url) File "remotePerfConfigurator.py", line 93, in convertUrlToRemote newline = newline.replace('webServer=', 'webServer=' + self.webserver); UnboundLocalError: local variable 'newline' referenced before assignment This one is actually a legitimate error i know how to fix

(In reply to Jeff Hammel [:jhammel] from comment #16) > Hmm, still getting the latter error on try: > > NOISE: [Exception... "Component returned failure code: 0x80520012 > (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init]" nsresult: "0x80520012 > (NS_ERROR_FILE_NOT_FOUND)" location: "JS frame :: > chrome://pageloader/content/pageloader.js :: plLoadURLsFromURI :: line 647" > data: no] > NOISE: > NOISE: __FAILbrowser frozen__FAIL > NOISE: [Exception... "Component returned failure code: 0x80520012 > (NS_ERROR_FILE_NOT_FOUND) [nsIFileInputStream.init]" nsresult: "0x80520012 > (NS_ERROR_FILE_NOT_FOUND)" location: "JS frame :: > chrome://pageloader/content/pageloader.js :: plLoadURLsFromURI :: line 647" > data: no] > NOISE: > NOISE: __FAILbrowser frozen__FAIL > NOISE: Found crashdump: > /tmp/tmpnaRhIo/profile/minidumps/0b08b020-0988-e352-32970a14-5bcb42e7.dmp > > I don't see this locally :( I've run with something much closer to how it is run on try: PerfConfigurator -v -e `which firefox` --results_url file://${HOME}/tprow.txt --activeTests tp5row --mozAfterPaint --responsiveness --filter ignore_first:5 --filter median --sampleConfig src/talos/talos/sample.2.config -o tp5row.yml --develop Still can't reproduce

pointer to the code: http://hg.mozilla.org/build/talos/file/61d1a38b5c75/talos/pageloader/chrome/pageloader.js#l647 It appears we're looking for a file that's not found. Why this causes a crash I do not know

this should green up android and a few other issues. I've also added tp5 to hgignore. We don't want to version this

Comment on attachment 618100 [details] [diff] [review] a few more fixes Review of attachment 618100 [details] [diff] [review]: ----------------------------------------------------------------- I like this patch...it cleans up a lot more!

Try run for 62991aca5c61 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=62991aca5c61 Results (out of 76 total builds): success: 52 failure: 24 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla.com-62991aca5c61

Try run for 0c69bc2b0ce9 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=0c69bc2b0ce9 Results (out of 76 total builds): exception: 4 success: 58 failure: 14 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla.com-0c69bc2b0ce9

(In reply to Mozilla RelEng Bot from comment #24) > Try run for 0c69bc2b0ce9 is complete. > Detailed breakdown of the results available here: > https://tbpl.mozilla.org/?tree=Try&rev=0c69bc2b0ce9 > Results (out of 76 total builds): > exception: 4 > success: 58 > failure: 14 > Builds (or logs if builds failed) available at: > http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla. > com-0c69bc2b0ce9 While there are numerous android failures most (all?) of them seem to be infrastructure based. This leaves tp5row, which I have yet to reproduce locally. I'll look at this closer and hopefully be able to figure something out

I think I found the issue: (talos)│hg diff diff --git a/talos/test.py b/talos/test.py --- a/talos/test.py +++ b/talos/test.py @@ -131,17 +131,17 @@ class tp4(tp): class tp4m(tp4): tpmanifest = '${talos}/page_load_test/tp4m.manifest' tpcycles = 2 win_counters = w7_counters = linux_counters = mac_counters = None remote_counters = ['Main_RSS', 'Content_RSS'] timeout = 14400 class tp5(tp): - tpmanifest = '${talos}/page_load_test/tp5.manifest' + tpmanifest = '${talos}/page_load_test/tp5/tp5.manifest' class tp5r(tp5): rss = True cycles = 1 responsiveness = False profile_path = '${talos}/base_profile' class tp5row(tp5): The reason I didn't have this issue locally is that I had tp5 manifest in the place where it was being looked for (as you can guess from my patch to .hgignore). Barring infrastructure failures, this should work now. I'll take a last look over the existing failures to make sure I didn't miss anything and update the patch and try again.

Going to push to try (again) and hope for the best

Comment on attachment 618481 [details] [diff] [review] add tpaint to keys Review of attachment 618481 [details] [diff] [review]: ----------------------------------------------------------------- lgtm

carrying r+ forward; i managed to break create_talos_zip.py with my .hgignore additions and got a completely red try run :/. This fixes that

posted to try: https://tbpl.mozilla.org/?tree=Try&rev=cda1a9bee5ac still waiting for rck2, but everything else seems green. There were some infra fails but rebuilds greened them up. If this goes green, i'll push, otherwise if you happen to read this :jmaher I'll push at your discretion

Try run for cda1a9bee5ac is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=cda1a9bee5ac Results (out of 86 total builds): success: 83 failure: 3 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jhammel@mozilla.com-cda1a9bee5ac

land this!

landed! :) http://hg.mozilla.org/build/talos/rev/3d1065e8e7c6

So now there is mostly minor cleanup to tend Talos towards a unified, semantic configuration model. The few things that (remote)PerfConfigurator does that are not intrinsic to creating a YAML file should be moveable pretty easily and the rest abstracted re https://bugzilla.mozilla.org/show_bug.cgi?id=704654. The goal is a program, invoked via (e.g.): talos -a active:tests -e `which firefox` --develop --other --options. This can run talos based on a configuration profile or, with --dump (https://bugzilla.mozilla.org/show_bug.cgi?id=719861) save said configuration to a .yml file. There's some ensuing cleanup bug bug 704654 is getting close.