Last Comment Bug 668869 - port ffox work to lazily load libxul to speed up start-up perf and remove wrapper startup script
: port ffox work to lazily load libxul to speed up start-up perf and remove wra...
Status: RESOLVED FIXED
[Linux port to complete]
: perf
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: Thunderbird 13.0
Assigned To: Mark Banner (:standard8, limited time in Dec)
:
:
Mentors:
: 673899 673900 730561 (view as bug list)
Depends on: 549246 552864 727406 762621
Blocks: tb-startupperf 451917 722262
  Show dependency treegraph
 
Reported: 2011-07-01 12:07 PDT by David :Bienvenu
Modified: 2012-06-11 05:46 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
wip (6.06 KB, patch)
2011-07-01 12:07 PDT, David :Bienvenu
no flags Details | Diff | Splinter Review
patch that links on windows (6.99 KB, patch)
2011-07-01 14:49 PDT, David :Bienvenu
no flags Details | Diff | Splinter Review
[Partially landed] attempt to fix linux build (6.02 KB, patch)
2011-08-22 11:40 PDT, David :Bienvenu
standard8: review+
Details | Diff | Splinter Review
Part 2 (2.82 KB, patch)
2011-09-20 13:24 PDT, Mark Banner (:standard8, limited time in Dec)
no flags Details | Diff | Splinter Review
Part 2 v2 (2.54 KB, patch)
2011-09-20 13:34 PDT, Mark Banner (:standard8, limited time in Dec)
no flags Details | Diff | Splinter Review
[Partially landed] Part 2 v3 (3.01 KB, patch)
2011-09-21 04:41 PDT, Mark Banner (:standard8, limited time in Dec)
mozilla: review+
Details | Diff | Splinter Review
[checked in] Finish port for Mac (2.41 KB, patch)
2011-10-25 02:56 PDT, Mark Banner (:standard8, limited time in Dec)
mozilla: review+
Details | Diff | Splinter Review
Linux port completion WIP (1.18 KB, patch)
2011-10-25 02:58 PDT, Mark Banner (:standard8, limited time in Dec)
no flags Details | Diff | Splinter Review
Finish port for Linux (15.04 KB, patch)
2012-02-20 03:19 PST, Mark Banner (:standard8, limited time in Dec)
gozer: review+
Details | Diff | Splinter Review

Description David :Bienvenu 2011-07-01 12:07:07 PDT
Created attachment 543484 [details] [diff] [review]
wip

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.
Comment 1 David :Bienvenu 2011-07-01 14:49:31 PDT
Created attachment 543518 [details] [diff] [review]
patch that links on windows

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.
Comment 2 David :Bienvenu 2011-08-16 10:04:20 PDT
Protz, this is the patch that adds the telemetry stuff to Thunderbird that you said you could give a spin on Linux.
Comment 3 David :Bienvenu 2011-08-22 08:48:28 PDT
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
Comment 4 David :Bienvenu 2011-08-22 11:40:59 PDT
Created attachment 554915 [details] [diff] [review]
[Partially landed] attempt to fix linux build

linux failed w/ try server (though mac succeeded). This patch is an attempt to fix the linux bustage.
Comment 5 David :Bienvenu 2011-08-22 16:40:29 PDT
Comment on attachment 554915 [details] [diff] [review]
[Partially landed] attempt to fix linux build

linux try server build happier with this patch
Comment 6 Mark Banner (:standard8, limited time in Dec) 2011-09-01 04:12:26 PDT
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.
Comment 7 Mark Banner (:standard8, limited time in Dec) 2011-09-20 13:24:26 PDT
Created attachment 561280 [details] [diff] [review]
Part 2

I've got to push this to try yet, but this should fix the additional executable renames that I was on about.
Comment 8 David :Bienvenu 2011-09-20 13:32:29 PDT
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.
Comment 9 Mark Banner (:standard8, limited time in Dec) 2011-09-20 13:34:44 PDT
Created attachment 561284 [details] [diff] [review]
Part 2 v2

Well it would do if I refreshed before uploading the patch.
Comment 10 Mark Banner (:standard8, limited time in Dec) 2011-09-21 04:39:27 PDT
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 ;-)
Comment 11 Mark Banner (:standard8, limited time in Dec) 2011-09-21 04:41:46 PDT
Created attachment 561437 [details] [diff] [review]
[Partially landed] Part 2 v3

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
Comment 12 David :Bienvenu 2011-09-21 06:57:19 PDT
Comment on attachment 561437 [details] [diff] [review]
[Partially landed] Part 2 v3

sure, r=me, since it works on try-server...
Comment 13 Mark Banner (:standard8, limited time in Dec) 2011-09-21 08:37:07 PDT
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.
Comment 14 Mark Banner (:standard8, limited time in Dec) 2011-09-21 13:52:04 PDT
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.
Comment 15 Mark Banner (:standard8, limited time in Dec) 2011-10-25 02:56:32 PDT
Created attachment 569317 [details] [diff] [review]
[checked in] Finish port for Mac

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
Comment 16 Mark Banner (:standard8, limited time in Dec) 2011-10-25 02:58:24 PDT
Created attachment 569319 [details] [diff] [review]
Linux port completion WIP

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.
Comment 17 David :Bienvenu 2011-10-25 09:42:28 PDT
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
Comment 18 Mark Banner (:standard8, limited time in Dec) 2011-10-25 12:30:30 PDT
Comment on attachment 569317 [details] [diff] [review]
[checked in] Finish port for Mac

Checked in: http://hg.mozilla.org/comm-central/rev/7582d11356ff
Comment 19 Serge Gautherie (:sgautherie) 2011-12-21 01:19:35 PST
(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.)
Comment 20 Mark Banner (:standard8, limited time in Dec) 2012-02-20 03:19:48 PST
Created attachment 598814 [details] [diff] [review]
Finish port for Linux

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.
Comment 21 Mike Conley (:mconley) 2012-02-21 11:45:43 PST
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
Comment 22 Mark Banner (:standard8, limited time in Dec) 2012-02-21 12:09:28 PST
Comment on attachment 598814 [details] [diff] [review]
Finish port for Linux

gozer, fancy taking a look?
Comment 23 Mike Hommey [:glandium] 2012-02-27 07:58:35 PST
*** Bug 730561 has been marked as a duplicate of this bug. ***
Comment 24 Mark Banner (:standard8, limited time in Dec) 2012-02-27 12:48:15 PST
Checked in the Linux part: http://hg.mozilla.org/comm-central/rev/8fecb303de4b
Comment 25 Lucas 2012-03-14 11:51:48 PDT
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.
Comment 26 Mark Banner (:standard8, limited time in Dec) 2012-03-15 02:01:45 PDT
(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.
Comment 27 Mike Hommey [:glandium] 2012-05-15 00:24:53 PDT
*** Bug 673899 has been marked as a duplicate of this bug. ***
Comment 28 Mike Hommey [:glandium] 2012-05-15 00:25:54 PDT
*** Bug 673900 has been marked as a duplicate of this bug. ***

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