Last Comment Bug 641614 - Omnijar ordering is not preserved by l10n repacking
: Omnijar ordering is not preserved by l10n repacking
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Windows 7
: -- major (vote)
: ---
Assigned To: Michael Wu [:mwu]
:
Mentors:
Depends on: 559961
Blocks: 641803
  Show dependency treegraph
 
Reported: 2011-03-14 13:02 PDT by (dormant account)
Modified: 2011-06-10 17:58 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
Fix (771 bytes, patch)
2011-04-14 17:47 PDT, Michael Wu [:mwu]
ted: review+
Details | Diff | Splinter Review

Description (dormant account) 2011-03-14 13:02:20 PDT
Work implemented in bug 559961 and bug 589368 did not get turned on in our RC1. I'm guessing this is because the build was done manually(according to https://bugzilla.mozilla.org/show_bug.cgi?id=641325#c2). Since there is no build log, I have no other leads. We shipped ordered omnijar in betas and it's working correctly in nightlies.
I hope FF4 release gets the omnijar ordering right.
Comment 1 Ben Hearsum (:bhearsum) 2011-03-14 13:06:49 PDT
"manually" just means "ran the steps that buildbot would run by hand", so I would be surprised if that's a culprit. Regardless, I'll help investigate this in a bit.
Comment 2 al_9x 2011-03-14 13:48:28 PDT
> Firefox 4 RC1 does not have an ordered omnijar

Not exactly, the windows en-us installer does, and has had for long time.  As you said, "Looks like our l10n repacks might be busted"

(In reply to comment #0)
> We shipped ordered omnijar in betas and it's
> working correctly in nightlies.

The windows German b12 does not have the ordered omnijar.  Possibly none of the localized installers do, or ever had.
Comment 3 Ben Hearsum (:bhearsum) 2011-03-14 13:50:28 PDT
Any idea if they were ordered at any point? Eg, an earlier beta?
Comment 4 Ben Hearsum (:bhearsum) 2011-03-14 13:54:46 PDT
cc'ing others who may have something to add.
Comment 5 Ben Hearsum (:bhearsum) 2011-03-14 13:57:10 PDT
Also, has anyone checked other platforms?
Comment 6 (dormant account) 2011-03-14 14:05:42 PDT
(In reply to comment #5)
> Also, has anyone checked other platforms?

This only works on PGO-enabled platforms, ie windows.

> > Firefox 4 RC1 does not have an ordered omnijar
> 
> Not exactly, the windows en-us installer does, and has had for long time.  As
> you said, "Looks like our l10n repacks might be busted"
> 
> (In reply to comment #0)
> > We shipped ordered omnijar in betas and it's
> > working correctly in nightlies.
> 
> The windows German b12 does not have the ordered omnijar.  Possibly none of the
> localized installers do, or ever had.


Thanks for extensive testing.


I just confirmed that rc is fine with en-US and broken in german and others. This is disappointing as I developed this while testing with russian locale which is broken now.(In reply to comment #2)
Comment 7 Ben Hearsum (:bhearsum) 2011-03-14 14:08:19 PDT
I did a quick look at b5 through b11 (all of the other releases that had omni.jar). None of their win32 de builds had an ordered omni.jar, which makes me strongly think that this never worked.
Comment 8 (dormant account) 2011-03-14 14:08:37 PDT
To summarize: non en-US builds wont get ordered omnijar in ff4 :(
Comment 9 Ben Hearsum (:bhearsum) 2011-03-14 14:22:23 PDT
Marking as blocking2.0? to make drivers aware of the fact that we don't have optimized omni.jar's for any l10n repacks. So far, it looks like we've never had it for l10n repacks.
Comment 10 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-03-14 14:31:00 PDT
Indeed, but it need not block.
Comment 11 Ben Hearsum (:bhearsum) 2011-03-14 14:56:23 PDT
I just did a couple more paranoid checks to confirm that this is l10n-only, and indeed, it is:
- Latest win32 nightly en-US build (zip and installer) -- ordered
- Latest l10n nightly de build (zip and installer) -- unordered
Comment 12 Ted Mielczarek [:ted.mielczarek] 2011-03-14 15:22:00 PDT
So, intuitively this makes sense if you think about it, because we optimize the omnijar using info in $(DIST)/jarlog:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#327

Now that command is part of $(MAKE_PACKAGE):
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#337

which does get called to repack the l10n builds:
http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/l10n.mk#151

...except $(DIST)/jarlog isn't going to exist on the machine that does the repacking.
Comment 13 (dormant account) 2011-03-14 15:25:46 PDT
UNPACK_OMNIJAR recreates the logdir, see http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#331
Comment 14 Michael Wu [:mwu] 2011-04-14 17:40:19 PDT
The perl command that we use to do s/en-US/AB_CD/g stomps all over the logs instead of doing what it's suppose to. I have no idea why. I'll fix/replace it.
Comment 15 Michael Wu [:mwu] 2011-04-14 17:47:05 PDT
Created attachment 526167 [details] [diff] [review]
Fix

It seems like perl doesn't do inplace editing well in windows. Having it make a backup seems to make this work better.
Comment 16 (dormant account) 2011-04-14 17:58:39 PDT
(In reply to comment #15)
> It seems like perl doesn't do inplace editing well in windows. Having it make a
> backup seems to make this work better.

ewww, thanks for catching this.
Comment 17 Ted Mielczarek [:ted.mielczarek] 2011-04-15 11:21:27 PDT
Comment on attachment 526167 [details] [diff] [review]
Fix

Boo, perl!
Comment 18 Michael Wu [:mwu] 2011-04-19 10:55:40 PDT
Optimistically resolving fixed.

http://hg.mozilla.org/mozilla-central/rev/9cf0970e50e5
Comment 19 Michael Wu [:mwu] 2011-04-20 13:48:23 PDT
Good news: The logs now exist and omni.jar in the ja repack does use an optimized jar.
Bad news: Localized files don't seem to be ordered. I can't reproduce it with my standard test setup - localized files are ordered fine there. Not sure what's going on.
Comment 20 Axel Hecht [:Pike] 2011-04-22 04:14:56 PDT
Inline editing won't work in general, as we're unpacking once, and then doing multiple repacks on it. At which point you couldn't replace en-US anymore, it's already 'af'. We'll need to keep the en-US jarlog around, and create a "localized" one on each run.

Reopening.

I'm a bit confused by my local tests, I see

+ cd firefox/Nightly.app/Contents/MacOS
+ /usr/bin/python2.6 /Users/axelhecht/src/central/mozilla-central/config/optimizejars.py --deoptimize /Users/axelhecht/src/central/b-repack/browser/locales/../../dist/jarlog/ ./ ./
Deoptimized 0/1479 in ./omni.jar


similar thing happens on the nightlies, linux log says:

./omni.jar: startup data ends at byte 0
Deoptimized 0/1450 in ./omni.jar

wokbok:dist axelhecht$ ls -l jarlog/
total 0
-rw-r--r--  1 axelhecht  staff  0 22 Apr 11:52 omni.jar.log

shows that omni.jar.log is empty after unpacking.

Debugging shows that readahead is 0 in my case, and thus
  elif readahead >= old_entry_offset + len(data):
never triggers? But that's all magic to me, I don't know what it's supposed to do.
Comment 21 Michael Wu [:mwu] 2011-04-22 08:58:39 PDT
(In reply to comment #20)
> Inline editing won't work in general, as we're unpacking once, and then doing
> multiple repacks on it.

Why wouldn't it work? AFAIK, it actually does work.

> At which point you couldn't replace en-US anymore, it's
> already 'af'. We'll need to keep the en-US jarlog around, and create a
> "localized" one on each run.
> 

Why? Doing the replacement each time is harmless - it just does nothing.

> Reopening.
> 
> I'm a bit confused by my local tests, I see
> 
> + cd firefox/Nightly.app/Contents/MacOS
> + /usr/bin/python2.6
> /Users/axelhecht/src/central/mozilla-central/config/optimizejars.py
> --deoptimize
> /Users/axelhecht/src/central/b-repack/browser/locales/../../dist/jarlog/ ./ ./
> Deoptimized 0/1479 in ./omni.jar
> 

Ordering only happens if you either do PGO or go out of your way to generate jarlogs. Linux/OSX omnijars are not ordered, so this behavior is expected. Windows is the only platform that we currently reorder jars on.

There is an issue with certain entries not getting ordered but I want to address that in a separate bug.
Comment 22 Michael Wu [:mwu] 2011-04-22 09:00:08 PDT
(In reply to comment #21)
> > At which point you couldn't replace en-US anymore, it's
> > already 'af'. We'll need to keep the en-US jarlog around, and create a
> > "localized" one on each run.
> > 
> 
> Why? Doing the replacement each time is harmless - it just does nothing.
> 
Oh, are you saying we unpack once and do multiple language repacks? Then this other issue makes sense. But I still want to address that in another bug.
Comment 23 Michael Wu [:mwu] 2011-06-10 17:58:27 PDT
Accidentally checked in the patch for bug 659379 with this bug's number. But, bug 659379 is the followup for this anyway, so good enough.

Note You need to log in before you can comment on or make changes to this bug.