Closed
Bug 722262
Opened 12 years ago
Closed 12 years ago
Port |Bug 552864 - Throw away wrapper shell script on unix and lazily load libxul| to SeaMonkey
Categories
(SeaMonkey :: Startup & Profiles, defect)
SeaMonkey
Startup & Profiles
Tracking
(seamonkey2.7 wontfix, seamonkey2.8 wontfix, seamonkey2.9 wontfix)
RESOLVED
FIXED
seamonkey2.10
People
(Reporter: sgautherie, Assigned: Callek)
References
(Blocks 1 open bug)
Details
Attachments
(3 files, 7 obsolete files)
22.78 KB,
patch
|
glandium
:
review+
mnyromyr
:
feedback+
|
Details | Diff | Splinter Review |
565 bytes,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
897 bytes,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
See also TB bug 668869.
Reporter | ||
Updated•12 years ago
|
Reporter | ||
Updated•12 years ago
|
status-seamonkey2.7:
--- → wontfix
status-seamonkey2.8:
--- → wontfix
status-seamonkey2.9:
--- → affected
Summary: Port |Bug 668869 - port ffox work to lazily load libxul to speed up start-up perf and remove wrapper startup script| to SeaMonkey → Port |Bug 552864 - Throw away wrapper shell script on unix and lazily load libxul| to SeaMonkey
Assignee | ||
Comment 1•12 years ago
|
||
I hope to do this this cycle (and have been tracking it to do so, just never got around to filing the bug, afaict) It is too risky to take on aurora though so wontfix-2.9
Assignee: nobody → bugspam.Callek
Reporter | ||
Updated•12 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•12 years ago
|
||
This is just a snapshot of where I am right now, I don't think this compiles (did not test yet) and I am pretty sure I still have a lot to do. Notes: * Yes I am adding features here at the same time (I think I need to turn on Telemetry for this code) * I need to add some confvars code to support the gen app-ini stuff. * Our Makefile for suite/app has a lot of cruft in it and will be (attempted) to be cleaned up here. * I will be trying to limit the scope of this all to the bare minimum to support what I did in suite/app/*.cpp here and cleaning up that Makefile, beyond that (moving stuff, changing how extensions are packaged to match Firefox's test-pilot, etc.) will be out of scope here.
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #594527 -
Attachment is obsolete: true
Assignee | ||
Comment 4•12 years ago
|
||
This is it for the night, this actually _does compile_ (on win anyway) and _does startup_. I did not test much else yet, but this looks like a good WIP first step.
Attachment #594652 -
Attachment is obsolete: true
Assignee | ||
Comment 5•12 years ago
|
||
Ok, so far as I can tell, this should do it -- accurately. I did not test on anything other than Win7, and I did add LIBXUL supporting code, even though we don't actively support it that way (yet). I'm HOPING for a fast review, since this is relatively risky enough to want it to land early in the cycle
Attachment #594658 -
Attachment is obsolete: true
Attachment #594861 -
Flags: review?(kairo)
Attachment #594861 -
Flags: feedback?(mnyromyr)
Comment 6•12 years ago
|
||
Comment on attachment 594861 [details] [diff] [review] v1.0 Review of attachment 594861 [details] [diff] [review]: ----------------------------------------------------------------- I'm marking this reviewed, though I didn't exactly look at every line of this, in summary this looks good though, and if it builds on all platforms, I think it's a good step. Thanks for this work!
Attachment #594861 -
Flags: review?(kairo) → review+
Assignee | ||
Comment 7•12 years ago
|
||
r+ from KaiRo, this has a few minor changes that should make a real difference for mac though
Attachment #594861 -
Attachment is obsolete: true
Attachment #594861 -
Flags: feedback?(mnyromyr)
Attachment #595981 -
Flags: review+
Attachment #595981 -
Flags: feedback?
Assignee | ||
Updated•12 years ago
|
Attachment #595981 -
Flags: feedback? → feedback?(mnyromyr)
Comment 8•12 years ago
|
||
Comment on attachment 595981 [details] [diff] [review] v1.5 This would break Linux builds - see bug 668869 comment 14.
Attachment #595981 -
Flags: review-
Assignee | ||
Comment 9•12 years ago
|
||
So following up and documenting here for sanity: Justin@ORION /d/Objects/SeaMonkeyTrunk/mozilla/xpcom/stub $ make echo-variable-DEPENDENT_LIBS_LIST nspr4.dll plc4.dll plds4.dll mozalloc.dll mozsqlite3.dll nssutil3.dll softokn3.dll nss3.dll ssl3.dl l smime3.dll mozjs.dll xul.dll Justin@ORION /d/Objects/SeaMonkeyTrunk/mozilla/xpcom/stub $ ls ../../dist/bin/*.dll ../../dist/bin/AccessibleMarshal.dll ../../dist/bin/nsldappr32v60.dll ../../dist/bin/D3DCompiler_43.dll ../../dist/bin/nsldif32v60.dll ../../dist/bin/IA2Marshal.dll ../../dist/bin/nspr4.dll ../../dist/bin/MapiProxy.dll ../../dist/bin/nss3.dll ../../dist/bin/crashinjectdll.dll ../../dist/bin/nssckbi.dll ../../dist/bin/d3dx9_43.dll ../../dist/bin/nssdbm3.dll ../../dist/bin/freebl3.dll ../../dist/bin/nssutil3.dll ../../dist/bin/gkmedias.dll ../../dist/bin/plc4.dll ../../dist/bin/libEGL.dll ../../dist/bin/plds4.dll ../../dist/bin/libGLESv2.dll ../../dist/bin/smime3.dll ../../dist/bin/mozMapi32.dll ../../dist/bin/softokn3.dll ../../dist/bin/mozalloc.dll ../../dist/bin/ssl3.dll ../../dist/bin/mozglue.dll ../../dist/bin/vmwarerecordinghelper.dll ../../dist/bin/mozjs.dll ../../dist/bin/xpcom.dll ../../dist/bin/mozsqlite3.dll ../../dist/bin/xul.dll ../../dist/bin/nsldap32v60.dll Justin@ORION /d/Objects/SeaMonkeyTrunk/mozilla/xpcom/stub $ pushd ../../dist/bin/; rm -f `make -C ../../xpcom/stub echo-variable-DEPENDENT_LIBS_LIST`; ls *.d ll; popd AccessibleMarshal.dll d3dx9_43.dll mozMapi32.dll nssckbi.dll D3DCompiler_43.dll freebl3.dll mozglue.dll nssdbm3.dll IA2Marshal.dll gkmedias.dll nsldap32v60.dll vmwarerecordinghelper.dll MapiProxy.dll libEGL.dll nsldappr32v60.dll xpcom.dll crashinjectdll.dll libGLESv2.dll nsldif32v60.dll
Assignee | ||
Comment 10•12 years ago
|
||
[seabld@cb-seamonkey-linux-01 bin]$ make -C ../../xpcom/stub echo-variable-DEPENDENT_LIBS_LIST make: Entering directory `/builds/slave/comm-cen-trunk-lnx/build/objdir/mozilla/xpcom/stub' libnspr4.so libplc4.so libplds4.so libmozalloc.so libmozsqlite3.so libnssutil3.so libsoftokn3.so libnss3.so libssl3.so libsmime3.so libxul.so make: Leaving directory `/builds/slave/comm-cen-trunk-lnx/build/objdir/mozilla/xpcom/stub' (I'm trimming out OS provided libs from the following, just for the record) [seabld@cb-seamonkey-linux-01 bin]$ readelf -d libxpcom.so | grep NEEDED 0x00000001 (NEEDED) Shared library: [libxul.so] 0x00000001 (NEEDED) Shared library: [libplds4.so] 0x00000001 (NEEDED) Shared library: [libplc4.so] 0x00000001 (NEEDED) Shared library: [libnspr4.so] 0x00000001 (NEEDED) Shared library: [libmozalloc.so] [seabld@cb-seamonkey-linux-01 bin]$ readelf -d libxul.so | grep NEEDED 0x00000001 (NEEDED) Shared library: [libsmime3.so] 0x00000001 (NEEDED) Shared library: [libssl3.so] 0x00000001 (NEEDED) Shared library: [libnss3.so] 0x00000001 (NEEDED) Shared library: [libnssutil3.so] 0x00000001 (NEEDED) Shared library: [libldap60.so] <!--X 0x00000001 (NEEDED) Shared library: [libprldap60.so] <!--X 0x00000001 (NEEDED) Shared library: [libldif60.so] <!--X 0x00000001 (NEEDED) Shared library: [libmozsqlite3.so] 0x00000001 (NEEDED) Shared library: [libplds4.so] 0x00000001 (NEEDED) Shared library: [libplc4.so] 0x00000001 (NEEDED) Shared library: [libnspr4.so] 0x00000001 (NEEDED) Shared library: [libmozalloc.so] (all others have merely duplicates of the above or sys libraries)
Assignee | ||
Comment 11•12 years ago
|
||
This will be used in v2 of the comm-central patch.
Attachment #596465 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 12•12 years ago
|
||
Attachment #595981 -
Attachment is obsolete: true
Attachment #596465 -
Attachment is obsolete: true
Attachment #595981 -
Flags: feedback?(mnyromyr)
Attachment #596465 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 13•12 years ago
|
||
Attachment #596467 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 14•12 years ago
|
||
Comment on attachment 596466 [details] [diff] [review] [c-c] v2 [Checked in: Comment 20] I'll accept a review from Mark or Mike here. The key review point is as it relates to dependentlibs. The rest of this patch was reviewed by KaiRo, but happy to have comments from either of you if you catch anything wrong. f? from Karsten for basic functionality testing on mac. and f? from ewong for extra-sanity-checking functionality testing on linux. [I won't block landing on the feedbacks]
Attachment #596466 -
Flags: review?(mh+mozilla)
Attachment #596466 -
Flags: review?(mbanner)
Attachment #596466 -
Flags: feedback?(mnyromyr)
Attachment #596466 -
Flags: feedback?(ewong)
Assignee | ||
Comment 15•12 years ago
|
||
reattached a corrected patch per glandium.
Attachment #596467 -
Attachment is obsolete: true
Attachment #596467 -
Flags: review?(mh+mozilla)
Attachment #596469 -
Flags: review?(mh+mozilla)
Comment 16•12 years ago
|
||
Ooh, I like the solution. Want to test it out before I give r+ but the idea looks great.
Comment 17•12 years ago
|
||
Comment on attachment 596466 [details] [diff] [review] [c-c] v2 [Checked in: Comment 20] Mac looks okay (rebuild/clobber).
Attachment #596466 -
Flags: feedback?(mnyromyr) → feedback+
Updated•12 years ago
|
Attachment #596469 -
Flags: review?(mh+mozilla) → review+
Comment 18•12 years ago
|
||
Comment on attachment 596466 [details] [diff] [review] [c-c] v2 [Checked in: Comment 20] Review of attachment 596466 [details] [diff] [review]: ----------------------------------------------------------------- Whitespace excluded, nsSuiteApp.cpp just becomes the same content as nsBrowserApp.cpp, so it's obviously good :) The corresponding changes to Makefile.in are ok, but I do wonder where some of the stuff that's not in browser/app/Makefile.in come from. The rest looks good. ::: suite/extradependlibs.mk @@ +6,5 @@ > +# Presume all mozilla/autoconf.mk is included and that it does not have > +# Direct access to c-c specific vars not present there. > + > +# We don't have access to MOZ_LDAP_XPCOM here, so cheat. > +ifneq (,$(findstring ldap,$(MOZ_APP_COMPONENT_LIBS))) $(filter mozldap,$(MOZ_APP_COMPONENT_LIBS)) would be better imho
Attachment #596466 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Updated•12 years ago
|
Whiteboard: [leave open after inbound merge]
Reporter | ||
Updated•12 years ago
|
Attachment #596469 -
Attachment description: [m-c] v1.1 - include stuff from app → [m-c] v1.1 - include stuff from app
[Checked in: Comment 19]
Comment 20•12 years ago
|
||
This landed: http://hg.mozilla.org/comm-central/rev/9eddb2d0b431 Hmm... Just built myself from current trunk in my Linux VM. Then: jens@holodoc:~/usr/local/seamonkey-central > ./seamonkey XPCOMGlueLoad error for file /home/jens/usr/local/seamonkey-central/libxpcom.so: libxul.so: cannot open shared object file: No such file or directory Couldn't load XPCOM. Same with seamonkey-bin, which is binary-equal to seamonkey (unlike firefox vs firefox-bin for the latest Nightly). Is it just me?
Reporter | ||
Updated•12 years ago
|
Attachment #596466 -
Attachment description: [c-c] v2 → [c-c] v2
[Checked in: Comment 20]
Reporter | ||
Updated•12 years ago
|
Whiteboard: [leave open after inbound merge]
Target Milestone: --- → seamonkey2.10
Assignee | ||
Comment 21•12 years ago
|
||
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #20) > This landed: > http://hg.mozilla.org/comm-central/rev/9eddb2d0b431 > > Hmm... Just built myself from current trunk in my Linux VM. Then: > > jens@holodoc:~/usr/local/seamonkey-central > ./seamonkey > XPCOMGlueLoad error for file > /home/jens/usr/local/seamonkey-central/libxpcom.so: > libxul.so: cannot open shared object file: No such file or directory > Couldn't load XPCOM. > > Same with seamonkey-bin, which is binary-equal to seamonkey (unlike firefox > vs firefox-bin for the latest Nightly). > > Is it just me? No not just you, Seems Standard8 noticed this as well, he may have a fix, but if not I'll back this out shortly.
Comment 22•12 years ago
|
||
Unfortunately the original patch didn't realise that dependentlibs.list was ordered and therefore would still break on Linux. This patch fixes the ordering by including the extra libs for the app first. It works for at least both SM & TB, so hopefully we'll be ok.
Attachment #597130 -
Flags: review?(mh+mozilla)
Attachment #597130 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 23•12 years ago
|
||
Thanks mark and kyle, pushed the fixed: http://hg.mozilla.org/mozilla-central/rev/b3a7561624f9
Reporter | ||
Updated•12 years ago
|
Attachment #597130 -
Attachment description: Fix ordering for extra app libs in dependentlibs.list → Fix ordering for extra app libs in dependentlibs.list
[Checked in: Comment 23]
Reporter | ||
Comment 24•12 years ago
|
||
I didn't look too precisely, but tbpl hasn't reported any test run since "Tue Feb 14 04:29:59 2012 PST", which was the checkin before this one. I assume this is not a result of bug 722187, which was later backed out, as it is js only. Then this bug seems a likely culprit: some builders may just have not (re)run since then, but (some) Windows test builders seem to have a stuck SeaMonkey, which probably need a human maintenance or something! Can you check these(/all) test builders?
Reporter | ||
Comment 25•12 years ago
|
||
(In reply to Serge Gautherie (:sgautherie) from comment #24) > I didn't look too precisely, but tbpl hasn't reported any test run since > "Tue Feb 14 04:29:59 2012 PST", which was the checkin before this one. Oh, that must be another regression to be fixed by bug 727258, hopefully. > Can you check these(/all) test builders? Actually, only 'cb-seamonkey-win32-02' seems affected, afaict from current http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey
Reporter | ||
Comment 26•12 years ago
|
||
(In reply to Serge Gautherie (:sgautherie) from comment #25) > Actually, only 'cb-seamonkey-win32-02' seems affected, afaict from current > http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey Ftr, 'cb-seamonkey-win32-01' had been affected too (earlier), but seems fixed now. And 'cb-seamonkey-win32-02' seems to (have) been fixed too (now).
Reporter | ||
Comment 27•12 years ago
|
||
(In reply to Serge Gautherie (:sgautherie) from bug 722262 comment #25) > (In reply to Serge Gautherie (:sgautherie) from bug 722262 comment #24) > > > I didn't look too precisely, but tbpl hasn't reported any test run since > > "Tue Feb 14 04:29:59 2012 PST", which was the checkin before this one. > > Oh, that must be another regression to be fixed by bug 727258, hopefully. I filed bug 722675.
Comment 28•12 years ago
|
||
> I filed bug 722675. I think you meant Bug 727675. SeaMonkey doesn't run on Android :P
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 29•12 years ago
|
||
Comment on attachment 596466 [details] [diff] [review] [c-c] v2 [Checked in: Comment 20] Clearing obsolete requests.
Attachment #596466 -
Flags: review?(mbanner)
Attachment #596466 -
Flags: feedback?(ewong)
Reporter | ||
Comment 30•12 years ago
|
||
Comment on attachment 596466 [details] [diff] [review] [c-c] v2 [Checked in: Comment 20] Review of attachment 596466 [details] [diff] [review]: ----------------------------------------------------------------- ::: suite/installer/package-manifest.in @@ -111,1 @@ > @BINPATH@/@MOZ_APP_NAME@ Ftr, this part was from bug 549246.
Reporter | ||
Updated•12 years ago
|
Reporter | ||
Updated•12 years ago
|
Flags: in-testsuite-
You need to log in
before you can comment on or make changes to this bug.
Description
•