Need to land scripts to run for PGO input. This is a tiny python server and some static data that we'll add in steps, so we can see any improvements or regressions from profiling.

run profileserver.py to heat up our code. Moves a bunch of mochitest python stuff into a common area, so we can share.

Comment on attachment 304681 [details] [diff] [review] works on linux >Index: testing/mochitest/Makefile.in >=================================================================== > runtests.pl: runtests.pl.in > $(PYTHON) $(MOZILLA_DIR)/config/Preprocessor.py \ > $(TEST_DRIVER_PPARGS) $(DEFINES) $(ACDEFINES) $^ > $@ > > runtests.py: runtests.py.in > $(PYTHON) $(MOZILLA_DIR)/config/Preprocessor.py \ > $(TEST_DRIVER_PPARGS) $(DEFINES) $(ACDEFINES) $^ > $@ > >-GARBAGE += runtests.pl runtests.py >+automation.py: $(MOZILLA_DIR)/build/pgo/automation.py.in >+ $(PYTHON) $(MOZILLA_DIR)/config/Preprocessor.py \ >+ $(TEST_DRIVER_PPARGS) $(DEFINES) $(ACDEFINES) $^ > $@ >+ Can you just s/MOZILLA_DIR/topsrcdir/ while you're here? >Index: testing/mochitest/runtests.py.in >=================================================================== Did your changes remove all of the preprocessor stuff? Could this become a normal .py file instead of a .py.in now? (I skimmed most of this file, since it was mostly code removal, might want to have Waldo take a second look at it.) >Index: build/pgo/Makefile.in >=================================================================== >+# The Original Code is Mozilla Communicator client code, released >+# March 31, 1998. Fix the ancient copyright headers here. >+# These go in _tests/ or _profiling/ so they need to go up an extra path segment >+AUTOMATION_PPARGS = \ >+ -DBROWSER_PATH=$(browser_path) \ >+ -DXPC_BIN_PATH=\"$(DIST)/bin\" \ >+ $(NULL) I don't understand the comment here, what is it referring to? >+automation.py: automation.py.in >+ $(PYTHON) $(MOZILLA_DIR)/config/Preprocessor.py \ >+ $(AUTOMATION_PPARGS) $(DEFINES) $(ACDEFINES) $^ > $@ >+ >+profileserver.py: profileserver.py.in >+ $(PYTHON) $(MOZILLA_DIR)/config/Preprocessor.py $^ > $@ >+ chmod +x $@ Maybe in Mozilla2 we can figure out how to do this stuff without having to preprocess all our Python scripts. >Index: build/pgo/automation.py.in >=================================================================== >+# Portions created by the Initial Developer are Copyright (C) 1998 >+# the Initial Developer. All Rights Reserved. Just fix the year here. >+ # These are generated in mozilla/build/Makefile.in >+#expand DIST_BIN = "./" + __XPC_BIN_PATH__ >+#expand IS_WIN32 = len("__WIN32__") != 0 >+#expand IS_MAC = __IS_MAC__ != 0 >+#ifdef IS_CYGWIN >+#expand IS_CYGWIN = __IS_CYGWIN__ == 1 >+#else >+IS_CYGWIN = False >+#endif >+ >+UNIXISH = not IS_WIN32 and not IS_MAC Could you file a followup on just making this use sys.platform or something instead of the preprocessor for platform detection? Aside from the dist/bin path and the browser binary path, we should be able to detect this stuff at runtime. >Index: build/pgo/profileserver.py.in >=================================================================== Is there any reason this needs to be a .py.in? I didn't see any substitutions happening. If it wants to run from the objdir, you could just nsinstall it there. Also, can you add a little comment at the top explaining what it does? r=me with those changes

> > Can you just s/MOZILLA_DIR/topsrcdir/ while you're here? > > >Index: testing/mochitest/runtests.py.in > >=================================================================== > > Did your changes remove all of the preprocessor stuff? Could this become a > normal .py file instead of a .py.in now? No, because relative imports don't work when one file has been preprocessed but the other hasn't. Irritating, so I will file a follow up for that. Same for other instances of this pattern you noticed. > I don't understand the comment here, what is it referring to? It is stray. will fix. > >Index: build/pgo/automation.py.in > >=================================================================== > >+# Portions created by the Initial Developer are Copyright (C) 1998 > >+# the Initial Developer. All Rights Reserved. > > Just fix the year here. > > > >+ # These are generated in mozilla/build/Makefile.in > >+#expand DIST_BIN = "./" + __XPC_BIN_PATH__ > >+#expand IS_WIN32 = len("__WIN32__") != 0 > >+#expand IS_MAC = __IS_MAC__ != 0 > >+#ifdef IS_CYGWIN > >+#expand IS_CYGWIN = __IS_CYGWIN__ == 1 > >+#else > >+IS_CYGWIN = False > >+#endif > >+ > >+UNIXISH = not IS_WIN32 and not IS_MAC > > Could you file a followup on just making this use sys.platform or something > instead of the preprocessor for platform detection? Aside from the dist/bin > path and the browser binary path, we should be able to detect this stuff at > runtime. Perl has problems detecting it's on Windows from msys. I don't know if Python is the same.

MozillaBuild's python is a real Windows python, not from msys, so it works fine. I don't really care about supporting cygwin anymore anyway, but cygwin Python identifies itself as cygwin.

not so much a fan of |Foo(argBegin + \n argEnd)| in MochitestServer.start -- mind changing that to either put the argument all on one line, or if you're hitting eighty characters, assign to a local first? "Adds MochiKit chrome tests to the profile" should include a trailing period. In bug 417075 I need to change |new RegExp('http://(.*?(:\\\\\\\\d+)?)/')| to accommodate userPass in proxied URLs. Mind changing that to the following while you're here so I can just revert runtests.py.in locally, if you don't mind mixing up the reason for the change a touch? new RegExp('http://(?:[^/@]*@)?(.*?(:\\\\\\\\d+)?)/') I kinda want the whitespace above "# write userChrome.css" to stay; the extra whitespace serves to delineate separate sections for the separate files being written out by the method. Unindent "# These are generated in mozilla/build/Makefile.in" if that doesn't break the preprocessor (I think it doesn't). Same for the profile setup #-delineated block. Use all-caps for the "Run The App" block. What's the purpose of the profileserver part? You can't substitute it for httpd.js because Mochitests rely on httpd.js-specific behaviors like handling ^headers^ and .sjs files. Have you actually run the entire set of Mochitests with this change?

(In reply to comment #6) > not so much a fan of |Foo(argBegin + \n argEnd)| in MochitestServer.start -- > mind changing that to either put the argument all on one line, or if you're > hitting eighty characters, assign to a local first? Fixed. > "Adds MochiKit chrome tests to the profile" should include a trailing period. > Fixed. > In bug 417075 I need to change |new RegExp('http://(.*?(:\\\\\\\\d+)?)/')| to > accommodate userPass in proxied URLs. Mind changing that to the following > while you're here so I can just revert runtests.py.in locally, if you don't > mind mixing up the reason for the change a touch? Unrelated, fix in bug 417075. > I kinda want the whitespace above "# write userChrome.css" to stay; the extra > whitespace serves to delineate separate sections for the separate files being > written out by the method. > > Unindent "# These are generated in mozilla/build/Makefile.in" if that doesn't > break the preprocessor (I think it doesn't). Same for the profile setup > #-delineated block. Fixed. > Use all-caps for the "Run The App" block. Fixed. > What's the purpose of the profileserver part? You can't substitute it for > httpd.js... The patch does not do that. profileserver is used for PGO builds, not unit tests.

Filed bug 418896 and bug 418896 for follow-ups.

(In reply to comment #9) > Filed bug 418896 and bug 418896 for follow-ups. > also bug 418894. :)