Last Comment Bug 772841 - optimizejars.py doesn't do anything
: optimizejars.py doesn't do anything
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on:
Blocks: 773171 481815 1195331
  Show dependency treegraph
 
Reported: 2012-07-11 07:13 PDT by Mike Hommey [:glandium]
Modified: 2015-08-17 07:58 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
+
fixed
+
fixed
+
fixed


Attachments
Pack omnijar when doing stage-package (3.15 KB, patch)
2012-07-11 07:23 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Pack omnijar when doing stage-package (4.25 KB, patch)
2012-07-11 10:26 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Pack omnijar when doing stage-package (4.24 KB, patch)
2012-07-11 11:41 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Revert client.mk part of changeset 6b60ebe2cae4 (1.01 KB, patch)
2012-07-12 07:51 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Use make package instead of make stage-package before profile run, but avoid package signing. (1.11 KB, patch)
2012-07-12 08:06 PDT, Mike Hommey [:glandium]
ted: review+
catlee: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Review

Description Mike Hommey [:glandium] 2012-07-11 07:13:53 PDT
optimizejars.py was broken by the change of rule used to prepare firefox for the profile run from package to stage-package in bug 481815. Stage-package doesn't do omnijar packing, so when the profile runs, there's no omnijar to log accesses to.
Comment 1 Brian R. Bondy [:bbondy] 2012-07-11 07:19:15 PDT
Which change specifically caused the regression?
Comment 2 Mike Hommey [:glandium] 2012-07-11 07:21:28 PDT
(In reply to Brian R. Bondy [:bbondy] from comment #1)
> Which change specifically caused the regression?

This:
-       $(MAKE) -C $(PGO_OBJDIR) package
+       $(MAKE) -C $(PGO_OBJDIR) stage-package
Comment 3 Mike Hommey [:glandium] 2012-07-11 07:23:46 PDT
Created attachment 641051 [details] [diff] [review]
Pack omnijar when doing stage-package
Comment 4 Brian R. Bondy [:bbondy] 2012-07-11 07:23:57 PDT
I don't think 481815 made any such change.
Comment 5 Mike Hommey [:glandium] 2012-07-11 07:28:39 PDT
(In reply to Brian R. Bondy [:bbondy] from comment #4)
> I don't think 481815 made any such change.

https://hg.mozilla.org/mozilla-central/rev/6b60ebe2cae4
bug 481815 comment 458
Comment 6 Brian R. Bondy [:bbondy] 2012-07-11 07:31:18 PDT
ah ok, I thought you meant it was part of the core maintenance service code.  That patch really should have landed in its own bug number.
Comment 7 Mike Hommey [:glandium] 2012-07-11 09:37:00 PDT
Comment on attachment 641051 [details] [diff] [review]
Pack omnijar when doing stage-package

Doesn't work :(
https://tbpl.mozilla.org/php/getParsedLog.php?id=13429714&tree=Try
Comment 8 Mike Hommey [:glandium] 2012-07-11 10:26:42 PDT
Created attachment 641101 [details] [diff] [review]
Pack omnijar when doing stage-package

This should work better, on top of avoiding zipping the omnijar twice like we currently do.

I'd push to try if try was working:
pushing to ssh://hg.mozilla.org/try
remote: Traceback (most recent call last):
remote:   File "/usr/local/bin/pash.py", line 4, in <module>
remote:     import hg_helper, ldap_helper
remote:   File "/usr/local/bin/hg_helper.py", line 4, in <module>
remote:     from ldap_helper import ldap_connect, get_ldap_attribute
remote:   File "/usr/local/bin/ldap_helper.py", line 50
remote:     if access_time < yesterday:
remote:                               ^
remote: IndentationError: unindent does not match any outer indentation level
Comment 9 Mike Hommey [:glandium] 2012-07-11 11:41:43 PDT
Created attachment 641143 [details] [diff] [review]
Pack omnijar when doing stage-package

Previous one was breaking b2g and mobile.
Comment 10 Mike Hommey [:glandium] 2012-07-11 12:52:40 PDT
Comment on attachment 641143 [details] [diff] [review]
Pack omnijar when doing stage-package

Dammit, it breaks osx.
Comment 11 Mike Hommey [:glandium] 2012-07-12 07:51:21 PDT
Created attachment 641462 [details] [diff] [review]
Revert client.mk part of changeset 6b60ebe2cae4

It's getting too late in the 14 release process to try to do the right thing. It takes too long to get it not to break mac builds.
Comment 12 Mike Hommey [:glandium] 2012-07-12 08:06:12 PDT
Created attachment 641472 [details] [diff] [review]
Use make package instead of make stage-package before profile run, but avoid package signing.
Comment 13 Ted Mielczarek [:ted.mielczarek] 2012-07-12 14:55:09 PDT
Comment on attachment 641472 [details] [diff] [review]
Use make package instead of make stage-package before profile run, but avoid package signing.

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

Would be nice if we could avoid the overhead of actually generating the final package file as well, but not critical.
Comment 14 Mike Hommey [:glandium] 2012-07-12 15:02:00 PDT
(In reply to Ted Mielczarek [:ted] from comment #13)
> Would be nice if we could avoid the overhead of actually generating the
> final package file as well, but not critical.

That's what i was trying to do, but ran out of time.
Comment 15 Mike Hommey [:glandium] 2012-07-12 15:04:41 PDT
http://hg.mozilla.org/mozilla-central/rev/87ff6a0cc45e
Comment 16 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-12 15:06:06 PDT
please nominate for uplift with risk assessment so we can evaluate whether we can still take this on beta.
Comment 17 Mike Hommey [:glandium] 2012-07-12 15:11:24 PDT
Comment on attachment 641472 [details] [diff] [review]
Use make package instead of make stage-package before profile run, but avoid package signing.

[Approval Request Comment]
Regression caused by a part of bug 481815
User impact if declined: Keeping cold startup performance regression we got in either FF12 or FF13 (not entirely clear which one)
Testing completed (on m-c, etc.): Tested on try with pgo ; try logs checked by catlee and myself for correctness. 
Risk to taking this patch (and alternatives if risky): Quite low. It's not introducing new code/functionality, it's restoring the use of code that was mistakenly disabled by a small change from bug 481815.
String or UUID changes made by this patch: None
Comment 18 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-13 08:01:46 PDT
Comment on attachment 641472 [details] [diff] [review]
Use make package instead of make stage-package before profile run, but avoid package signing.

restoring use of broken code, go ahead and land to branches.

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