The build process should be able to notify firefox when executing a build for pgo purposes. Then the jar component can output jar-file-order info and jars could be repacked on the second build pass.

This is an extension of mwu's hack. It reorders the jar index(for easier debugging) and places the index at the front of the file

Rewrote the C tool in python so it integrated better into build system. All that is left is for pgo to set MOZ_JAR_LOG_DIR=/dist/jarlog/ to turn this on.

If that's going to be part of pgo, wouldn't it be better if the code generating the log was #ifdef'ed, so that it is not built on the second pass?

(In reply to comment #5) > If that's going to be part of pgo, wouldn't it be better if the code generating > the log was #ifdef'ed, so that it is not built on the second pass? No, I'd also like extensions to use this as a profiling mechanism so they can ship optimized jars.

Comment on attachment 464974 [details] [diff] [review] This should order jars on PGO This patch orders jars according to how they are used. This also deviates from the typical jar layout by sticking the central directory at the front(the jar is still backwards compatible with other zip tools). With these two changes jars can now be read in a linear pattern yielding over 2x reduction in pagefaults.

Looks like my profileserver.py change wasn't right. This doesn't quite generate logs during pgo(at least according to try server)

Comment on attachment 464974 [details] [diff] [review] This should order jars on PGO So, looking through at this, the one thing that I don't understand is why we need to do this during PGO. Can we do this at packaging time? It seems to me that you could build an instrumented version of libjar and drop that in a build and start it up and gather the information there. If it is done as part of PGO this raises all sorts of hairy l10n repack issues.

We don't reorder on pgo, the logs are supposed to be gathered during pgo. They are then consulted during make package.

(In reply to comment #12) > We don't reorder on pgo, the logs are supposed to be gathered during pgo. They > are then consulted during make package. My point is that ideally we'd gather the logs during packaging because - It really has no relevance to the actual compiler PGO - And doing the logging at packaging time will make it much easier for localized builds to benefit.

(In reply to comment #13) > My point is that ideally we'd gather the logs during packaging because > - It really has no relevance to the actual compiler PGO > - And doing the logging at packaging time will make it much easier for > localized builds to benefit. AFAIU, you just can't gather the logs during packaging, because you need to know in what order the files are accessed *at runtime*. Normal packaging doesn't run anything. What could be done, though, is to have an order file in the build tree, and have the PGO script check that it's not too far from the actual order. The file could then occasionally be updated in the tree, when it makes sense (before a release, after massive changes, etc.)

I don't see why we can't fire up the browser (or whatever) at packaging time.

Well, for one, because not everybody building the browser has the possibility to do so (think linux autobuilders without X11 ; though if they are going to run pgo or reftest/mochitest, they'll have to, but not everybody does).

I would like to land this post-beta4 to get some testing in the nightlies. This works with or without omnijar. Reason logs are gathered during pgo is because it's tricky to run the browser during packaging. Since pgo already runs the browser, it's a natural place to get a "natural" trace of our io patterns. I'd like to tackle l10n repacks in a separate bug so this code can land and get tested on our nightlies prior to b5. I think the code here is pretty simple. I'm mainly curious whether optimizejars.py should go elsewhere in the tree.

Comment on attachment 466399 [details] [diff] [review] order jars on pgo style nit, please add a newline after "namespace mozilla {" and before the closing namespace brace. Also "} // namespace mozilla" for clarity.

Addressed review comment. Carrying over r+

This should block ff4 as it's a significant perf boost for omnijar.

Comment on attachment 466737 [details] [diff] [review] order jars on pgo [Checkin: Comment 28] I believe we need this patch to take full advantage of omnijar.

We don't actually do PGO runs on non-Windows, right? How are you going to actually get this hooked up with Linux/Mac builds?

(In reply to comment #22) > We don't actually do PGO runs on non-Windows, right? How are you going to > actually get this hooked up with Linux/Mac builds? It looks like Taras is trying to get at least Linux builds do PGO for 4.0, and from the bugs I'm following it looks like it may happen. No idea for Mac, but it would still be a good idea imho to implement something like the 2nd paragraph in comment 14.

(In reply to comment #22) > We don't actually do PGO runs on non-Windows, right? How are you going to > actually get this hooked up with Linux/Mac builds? We'll get Linux as soon as we can deploy GCC 4.5, bug 559964. Not sure what the build-team holdup there is atm. On Mac we can start running pgo script just for this if we deem this to be important.

Can you please file a releng bug to that effect?

Pushed the config.mk fix to comm-central as well, looks like we had busted packaging due to the var not existing. http://hg.mozilla.org/comm-central/rev/9c4c87dd8bc9

In xulrunner/app/nsRegisterGREUnix.cpp: #include <mozilla/FileUtils.h> should be #include "mozilla/FileUtils.h"

http://hg.mozilla.org/mozilla-central/rev/32d403bf5a7a http://hg.mozilla.org/mozilla-central/rev/dde9bc692553

To fix compilation failure on Fedora 13 x86_64 at xulrunner/app/nsRegisterGREUnix.cpp

The l10n repacks run in the srcdir, which makes topsrcdir a relative path. Explicitly core_abspath that.

(In reply to comment #29) > Created attachment 467378 [details] [diff] [review] > fix? > > To fix compilation failure on Fedora 13 x86_64 at > xulrunner/app/nsRegisterGREUnix.cpp Takanori, is the using namespace mozilla; required? Can we qualify the names instead?

Comment on attachment 467405 [details] [diff] [review] abspath topsrcdir [Checkin: Comment 32] Landed, http://hg.mozilla.org/mozilla-central/rev/a7874c76431f

(In reply to comment #32) > Landed, http://hg.mozilla.org/mozilla-central/rev/a7874c76431f 1) I think you'll need to apply that change in js/ as well or things will break, 2) Could you apply that core_abspath change to comm-central/config/config.mk as well? rs=me for that. Thanks.

I did check js/src/config.mk, that looks pretty different already and doesn't have optimizejars.py at all. And I haven't pushed to comm-central in ages, no idea what the infrastructure for that looks like these days. I'd rather have someone else patch and push there.

The abspath trick is needed on comm-central as well for L10n repacks, in that case.

Comment on attachment 467458 [details] [diff] [review] do the abspath trick on c-c as well [Checkin: Comment 38] Pushed as http://hg.mozilla.org/comm-central/rev/6689958f1d25

(In reply to comment #31) > (In reply to comment #29) > Takanori, is the using namespace mozilla; required? Can we qualify the names > instead? As per bug 588783, "using namespace mozilla" is required.

(In reply to comment #39) > (In reply to comment #31) > > (In reply to comment #29) > > Takanori, is the using namespace mozilla; required? Can we qualify the names > > instead? > > As per bug 588783, "using namespace mozilla" is required. Actually the patch in bug 588783 uses qualified names.

Update my patch with merging attachment 467417 [details] [diff] [review] in bug 588783.

(In reply to comment #41) > Created attachment 467617 [details] [diff] [review] > fix? v2 > > Update my patch with merging attachment 467417 [details] [diff] [review] in bug 588783. Why did you decide to not do using namespace mozilla?

(In reply to comment #43) > (In reply to comment #41) > > Created attachment 467617 [details] [diff] [review] [details] > > fix? v2 > > > > Update my patch with merging attachment 467417 [details] [diff] [review] [details] in bug 588783. > Why did you decide to not do using namespace mozilla? Probably because of comment 31. :-P Did we ever establish good rules on using c++ namespaces. I remember .platform threads on them but never any concrete rules.

(In reply to comment #44) > Probably because of comment 31. :-P Yes. and comment #40. I don't know which is the better approach to fix, attachment 467378 [details] [diff] [review] or attachment 467617 [details] [diff] [review].

(In reply to comment #45) > (In reply to comment #44) > > Probably because of comment 31. :-P > > Yes. and comment #40. > > I don't know which is the better approach to fix, attachment 467378 [details] [diff] [review] or > attachment 467617 [details] [diff] [review]. Neither of those seems like a good reason. I already r+ed a using namespace mozilla patch, that should be enough.

(In reply to comment #44) > (In reply to comment #43) > > (In reply to comment #41) > > > Created attachment 467617 [details] [diff] [review] [details] [details] > > > fix? v2 > > > > > > Update my patch with merging attachment 467417 [details] [diff] [review] [details] [details] in bug 588783. > > Why did you decide to not do using namespace mozilla? > > Probably because of comment 31. :-P Circular argument! :-( > Did we ever establish good rules on using c++ namespaces. I remember .platform > threads on them but never any concrete rules. The rules are being followed all the time. Don't put using namespace foo; in a .h file. Do use it in .cpp files to avoid :: eyesores. What's the question? /be

(In reply to comment #47) > (In reply to comment #44) > > (In reply to comment #43) > > > (In reply to comment #41) > > > > Created attachment 467617 [details] [diff] [review] [details] [details] [details] > > > > fix? v2 > > > > > > > > Update my patch with merging attachment 467417 [details] [diff] [review] [details] [details] [details] in bug 588783. > > > Why did you decide to not do using namespace mozilla? > > > > Probably because of comment 31. :-P > Circular argument! :-( My point exactly. > > Did we ever establish good rules on using c++ namespaces. I remember .platform > > threads on them but never any concrete rules. > The rules are being followed all the time. Don't put using namespace foo; in a > .h file. Do use it in .cpp files to avoid :: eyesores. What's the question? > /be That was my question. Thanks!

Thank you everyone. I'd like to choose attachment 467378 [details] [diff] [review]. Do I have to re-apply attachment 467378 [details] [diff] [review] here? I have no way to cancel obsolete flag.

(In reply to comment #49) > Thank you everyone. > > I'd like to choose attachment 467378 [details] [diff] [review]. > Do I have to re-apply attachment 467378 [details] [diff] [review] here? > > I have no way to cancel obsolete flag. You have to click on Details, then the Edit link. Hardly user-friendly ;-)

(In reply to comment #50) Oh, thanks!

(In reply to comment #35) > Created attachment 467411 [details] [diff] [review] > doh, js, too :-( I found this has been already applied http://hg.mozilla.org/mozilla-central/rev/7fe781556c55

Pushed Takanori's fix. http://hg.mozilla.org/mozilla-central/rev/f0af50ab4215

Ginn, I don't know what's wrong with your setup, but everytime you touch a bug in any way, you're changing the OS to WinCE.

I'm curious, too. It happened several times, but not every time. I'll be more caution. Sorry for the spam.

(In reply to comment #9) > Comment on attachment 464974 [details] [diff] [review] > This patch orders jars according to how they are used. This also deviates from > the typical jar layout by sticking the central directory at the front(the jar > is still backwards compatible with other zip tools). This is NOT compatible with some other archive applications. This breaks 7zip - a widely used free Windows archive tool. See https://sourceforge.net/tracker/?func=detail&aid=3065694&group_id=14481&atid=114481.

This also breaks jar: /Applications/Nightly.app/Contents/MacOS $ jar tf omni.jar java.util.zip.ZipException: invalid CEN header (bad signature) at java.util.zip.ZipFile.open(Native Method) at java.util.zip.ZipFile.<init>(ZipFile.java:127) at java.util.zip.ZipFile.<init>(ZipFile.java:88) at sun.tools.jar.Main.list(Main.java:979) at sun.tools.jar.Main.run(Main.java:224) at sun.tools.jar.Main.main(Main.java:1149)