optimizejars.py doesn't do anything

RESOLVED FIXED in Firefox 14

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks: 1 bug)

unspecified
mozilla16
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox13 affected, firefox14+ fixed, firefox15+ fixed, firefox16+ fixed)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
status-firefox13: --- → affected
status-firefox14: --- → affected
status-firefox15: --- → affected
status-firefox16: --- → affected
Which change specifically caused the regression?
(Assignee)

Comment 2

5 years ago
(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
(Assignee)

Comment 3

5 years ago
Created attachment 641051 [details] [diff] [review]
Pack omnijar when doing stage-package
Attachment #641051 - Flags: review?(ted.mielczarek)
I don't think 481815 made any such change.
(Assignee)

Comment 5

5 years ago
(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
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.
(Assignee)

Comment 7

5 years ago
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
Attachment #641051 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 8

5 years ago
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
Attachment #641101 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #641051 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
Created attachment 641143 [details] [diff] [review]
Pack omnijar when doing stage-package

Previous one was breaking b2g and mobile.
Attachment #641143 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #641101 - Attachment is obsolete: true
Attachment #641101 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 10

5 years ago
Comment on attachment 641143 [details] [diff] [review]
Pack omnijar when doing stage-package

Dammit, it breaks osx.
Attachment #641143 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Blocks: 773171
(Assignee)

Comment 11

5 years ago
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.
Attachment #641462 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

5 years ago
Attachment #641143 - Attachment is obsolete: true
(Assignee)

Comment 12

5 years ago
Created attachment 641472 [details] [diff] [review]
Use make package instead of make stage-package before profile run, but avoid package signing.
Attachment #641472 - Flags: review?(ted.mielczarek)
Attachment #641472 - Flags: review?(catlee)
(Assignee)

Updated

5 years ago
Attachment #641462 - Attachment is obsolete: true
Attachment #641462 - Flags: review?(ted.mielczarek)

Updated

5 years ago
tracking-firefox14: --- → ?
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?

Updated

5 years ago
Attachment #641472 - Flags: review?(catlee) → review+
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.
Attachment #641472 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Comment 14

5 years ago
(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.
(Assignee)

Comment 15

5 years ago
http://hg.mozilla.org/mozilla-central/rev/87ff6a0cc45e
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
please nominate for uplift with risk assessment so we can evaluate whether we can still take this on beta.
tracking-firefox14: ? → +
tracking-firefox15: ? → +
tracking-firefox16: ? → +
(Assignee)

Comment 17

5 years ago
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
Attachment #641472 - Flags: approval-mozilla-release?
Attachment #641472 - Flags: approval-mozilla-beta?
Attachment #641472 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

5 years ago
Attachment #641472 - Flags: approval-mozilla-release?
(Assignee)

Updated

5 years ago
status-firefox16: affected → fixed
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.
Attachment #641472 - Flags: approval-mozilla-beta?
Attachment #641472 - Flags: approval-mozilla-beta+
Attachment #641472 - Flags: approval-mozilla-aurora?
Attachment #641472 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 19

5 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/156d908eccd4
http://hg.mozilla.org/releases/mozilla-beta/rev/20b57236b6ec
status-firefox14: affected → fixed
status-firefox15: affected → fixed

Updated

2 years ago
Blocks: 1195331
You need to log in before you can comment on or make changes to this bug.