Closed
Bug 668869
Opened 13 years ago
Closed 13 years ago
port ffox work to lazily load libxul to speed up start-up perf and remove wrapper startup script
Categories
(Thunderbird :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 13.0
People
(Reporter: Bienvenu, Assigned: standard8)
References
(Blocks 1 open bug)
Details
(Keywords: perf, Whiteboard: [Linux port to complete])
Attachments
(4 files, 5 obsolete files)
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
gozer
:
review+
|
Details | Diff | Splinter Review |
Thunderbird equivalent of bug 552864. Attached patch doesn't compile, but shows some of the direction. I think I need to make more changes to the mail/app Makefile.
Reporter | ||
Comment 1•13 years ago
|
||
haven't run this yet, but it gets a bit further in the build.
I haven't dealt with the linux shell wrapper issues; I'm trying to get windows building first.
Assignee: nobody → dbienvenu
Attachment #543484 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Reporter | ||
Comment 2•13 years ago
|
||
Protz, this is the patch that adds the telemetry stuff to Thunderbird that you said you could give a spin on Linux.
Reporter | ||
Comment 3•13 years ago
|
||
I've requested a try server build to see if mac/linux build and run with this patch - http://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bienvenu@nventure.com-1f3fab21b043
Reporter | ||
Comment 4•13 years ago
|
||
linux failed w/ try server (though mac succeeded). This patch is an attempt to fix the linux bustage.
Attachment #543518 -
Attachment is obsolete: true
Reporter | ||
Comment 5•13 years ago
|
||
Comment on attachment 554915 [details] [diff] [review]
[Partially landed] attempt to fix linux build
linux try server build happier with this patch
Attachment #554915 -
Flags: review?(mbanner)
Updated•13 years ago
|
Blocks: tb-startupperf
Keywords: perf
Assignee | ||
Comment 6•13 years ago
|
||
Comment on attachment 554915 [details] [diff] [review]
[Partially landed] attempt to fix linux build
I think, in particular, we need to also port the parts of attachment 539437 [details] [diff] [review], specifically the additional changes to browser/app/Makefile.in and the change to browser/app/macbuild/Contents/Info.plist.in - otherwise we'll still be using the wrapper script to start up.
Otherwise this looks good so far.
Attachment #554915 -
Flags: review?(mbanner) → review-
Assignee | ||
Comment 7•13 years ago
|
||
I've got to push this to try yet, but this should fix the additional executable renames that I was on about.
Reporter | ||
Comment 8•13 years ago
|
||
ah, yes, I was going to pick this up again. If you get a try server build, Pascal might be a good test case for this since I believe he's seeing the startup perf issue. He said ffox used to have this issue on his linux laptop but doesn't anymore.
Assignee | ||
Comment 9•13 years ago
|
||
Well it would do if I refreshed before uploading the patch.
Attachment #561280 -
Attachment is obsolete: true
Assignee | ||
Comment 10•13 years ago
|
||
Comment on attachment 554915 [details] [diff] [review]
[Partially landed] attempt to fix linux build
Given my part 2 patch for this, I'm now happy with the changes here, if you're happy with my part 2 patch ;-)
Attachment #554915 -
Flags: review- → review+
Assignee | ||
Comment 11•13 years ago
|
||
Minor tweak to address a couple of review nits from part 1.
This has passed try server builds, and fixes the current trunk bustage, hence I'd like to land it (I strongly suspect a subset of this is capable of fixing the bustage, something to do with the linking, but seeing as this is ready, lets land it).
http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=43de3ff60b6e
Attachment #561284 -
Attachment is obsolete: true
Attachment #561437 -
Flags: review?(dbienvenu)
Reporter | ||
Comment 12•13 years ago
|
||
Comment on attachment 561437 [details] [diff] [review]
[Partially landed] Part 2 v3
sure, r=me, since it works on try-server...
Attachment #561437 -
Flags: review?(dbienvenu) → review+
Assignee | ||
Comment 13•13 years ago
|
||
Ok, this isn't actually quite right and breaks Linux builds. Still figuring out if there's a subset we can land to fix the bustage, or if we have to fix the issue that breaks the Linux builds as well.
Assignee | ||
Comment 14•13 years ago
|
||
The combined patches didn't work fully for Linux - if you remove the start up script, then you need dependentlibs.list in your build and filling out for the extra ldap libs.
So I did a try server build that is a subset of the combined patches, which passed both try server and Linux locally, I've therefore pushed that to the tree:
http://hg.mozilla.org/comm-central/rev/11e47caa27fa
This should get the tree green again.
I'll post the rest of the combined patches tomorrow and note what needs to be done to complete this.
Assignee | ||
Comment 15•13 years ago
|
||
This completes the Mac part of the port. This won't actually buy us anything in terms of performance, but completes the port anyway. I'm also semi keen to do this so we can get the executable as just "thunderbird" rather than "thunderbird-bin" so that if people are using it direct, shortcuts etc can be updated as appropriate.
Try server builds coming here:
http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=24f27b50419d
Assignee: dbienvenu → mbanner
Attachment #569317 -
Flags: review?(dbienvenu)
Assignee | ||
Comment 16•13 years ago
|
||
This "completes" the port of this bug, but still doesn't work as we need to hack dependentlibs.list to add our specific files. Posting here for now so it doesn't get lost.
Reporter | ||
Comment 17•13 years ago
|
||
Comment on attachment 569317 [details] [diff] [review]
[checked in] Finish port for Mac
Review of attachment 569317 [details] [diff] [review]:
-----------------------------------------------------------------
probably want to post to m.d.a.t. so developers will know to debug thunderbird, not thunderbird-bin
Attachment #569317 -
Flags: review?(dbienvenu) → review+
Assignee | ||
Updated•13 years ago
|
Attachment #561437 -
Attachment description: Part 2 v3 → [Partially landed] Part 2 v3
Assignee | ||
Updated•13 years ago
|
Attachment #554915 -
Attachment description: attempt to fix linux build → [Partially landed] attempt to fix linux build
Assignee | ||
Comment 18•13 years ago
|
||
Comment on attachment 569317 [details] [diff] [review]
[checked in] Finish port for Mac
Checked in: http://hg.mozilla.org/comm-central/rev/7582d11356ff
Attachment #569317 -
Attachment description: Finish port for Mac → [checked in] Finish port for Mac
Assignee | ||
Updated•13 years ago
|
Whiteboard: [Linux port to complete]
Comment 19•13 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #18)
> Checked in: http://hg.mozilla.org/comm-central/rev/7582d11356ff
Ftr, package-manifest.in part was from bug 549246.
(At least from bug 506493 comment 51 point of view.)
Depends on: 549246
Version: unspecified → Trunk
Assignee | ||
Comment 20•13 years ago
|
||
Note: this needs the patch from bug 727406 applying first to make it work properly.
This is the patch that will complete this bug, it should in theory provide a bit of startup perf increase for Linux, but in any case, it'll match our code to Firefox's.
There is a try server build coming here:
http://hg.mozilla.org/try-comm-central/rev/0feda937a588
What this does:
- Sync mail/app/Makefile.in with the browser equivalent as much as possible. The diffs that remain are 1) application differences 2) the fact we still have a separate application.ini file rather than a predefined struct (I'll consider dealing with that in a follow-up), 3) diffs due to bug 726975 not having landed yet, 4) some diffs due to how we do branding.
- Sync mail/app/nsMailApp.cpp with nsBrowserApp.cpp, the diffs being the separate application.ini file, and that we don't allow xulrunner style startup on the Thunderbird exe.
Functionally:
- We drop the script for "thunderbird" and replace it with an executable the same as thunderbird-bin. We don't drop thunderbird-bin for backwards compatibility (some places thunderbird-bin is still used apparently), and the updater can't cope with symlinks.
- We drop compiling thunderbird against some libs it doesn't actually need to be compiled against, because libxul compiles against those.
- Port bug 582910 to reduce the diffs and we'll need it for 64 bit builds probably
- Drops old debug code and static build items from the splash.rc files (Firefox no longer has these)
- Drops the now obsolete MOZ_STATIC_BUILD_UNSUPPORTED flag
- Adds the LDAP libs to the dependentlibs.list file and packages the file.
Attachment #569319 -
Attachment is obsolete: true
Attachment #598814 -
Flags: review?(mconley)
Comment 21•13 years ago
|
||
Comment on attachment 598814 [details] [diff] [review]
Finish port for Linux
Review of attachment 598814 [details] [diff] [review]:
-----------------------------------------------------------------
Hey Mark,
This looks OK to me, but I'm not sure I'm the right guy to review anything but the nsMailApp.cpp and all-thunderbird.js. While I understand how Makefiles work, and can recognize a syntax error when I see one, a lot of our build system is voodoo to me. I doubt I could pick up a defect if one was in there.
So I'm hesitant to give it an r+, since I can only really do an effective job on a subset of the files.
nsMailApp.cpp and all-thunderbird.js look OK to me though.
-Mike
Assignee | ||
Comment 22•13 years ago
|
||
Comment on attachment 598814 [details] [diff] [review]
Finish port for Linux
gozer, fancy taking a look?
Attachment #598814 -
Flags: review?(mconley) → review?(gozer)
Updated•13 years ago
|
Attachment #598814 -
Flags: review?(gozer) → review+
Assignee | ||
Comment 24•13 years ago
|
||
Checked in the Linux part: http://hg.mozilla.org/comm-central/rev/8fecb303de4b
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 13.0
Comment 25•13 years ago
|
||
Target Milestone: --- → Thunderbird 13.0
Is this for windows platform only or for Linux also ?
I've just tried Thunderbird 11 for Linux x86-64 - nothing changed.
Assignee | ||
Comment 26•13 years ago
|
||
(In reply to Lucas from comment #25)
> Target Milestone: --- → Thunderbird 13.0
>
> Is this for windows platform only or for Linux also ?
This change will only affect Linux.
> I've just tried Thunderbird 11 for Linux x86-64 - nothing changed.
Correct, that is why the Target Milestone is set to 13 as you've already highlighted. Note that we cannot guarantee the size of the startup speed increase. We've not tried measuring it, but we do know that Firefox saw improvements with these changes.
OS: Windows 7 → Linux
Hardware: x86 → All
You need to log in
before you can comment on or make changes to this bug.
Description
•