Closed
Bug 207407
Opened 21 years ago
Closed 21 years ago
Enable building a linkable library of gecko components
Categories
(SeaMonkey :: Build Config, defect)
SeaMonkey
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bryner, Assigned: bryner)
Details
(Keywords: perf)
Attachments
(1 file, 1 obsolete file)
18.21 KB,
patch
|
ccarlen
:
review+
sfraser_bugs
:
superreview+
|
Details | Diff | Splinter Review |
Some platforms (Mac) can get a major startup time boost from being able to prebind libraries, which requires having all of the symbols present at link time. I came up with an approach that allows us to do this by creating a linkable library containing all (well, whichever ones you build) of the gecko components, which can then be registered via the static component loader. Your application or embedding wrapper would link directly against this library. I did some measurements on Mac OS X (10.2.6) and found that I got around 35% startup time improvement using this approach with mozilla-bin.
Assignee | ||
Comment 1•21 years ago
|
||
note: I didn't actually add the directory embedding/componentlib to CVS, in case reviewers think another location would be better. I just diff'd the files there against /dev/null and appended it to the patch, hopefully it will apply.
Assignee | ||
Comment 2•21 years ago
|
||
oops, this wasn't supposed to include the embedding/browser/cocoa change.
Attachment #124421 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #124422 -
Flags: review?(seawood)
Updated•21 years ago
|
Attachment #124422 -
Flags: review?(seawood)
Assignee | ||
Updated•21 years ago
|
Attachment #124422 -
Flags: superreview?(sfraser)
Attachment #124422 -
Flags: review?(ccarlen)
Comment 3•21 years ago
|
||
Comment on attachment 124422 [details] [diff] [review] patch >Index: browser/app/Makefile.in >=================================================================== >RCS file: /cvsroot/mozilla/browser/app/Makefile.in,v Is this part of this patch? >Index: config/config.mk >=================================================================== >RCS file: /cvsroot/mozilla/config/config.mk,v >retrieving revision 3.270 >diff -u -r3.270 config.mk >--- config/config.mk 16 May 2003 06:07:35 -0000 3.270 >+++ config/config.mk 29 May 2003 06:23:51 -0000 >@@ -695,6 +695,18 @@ > endif > endif > >+# Flags needed to link against the component library >+ifdef MOZ_COMPONENTLIB >+MOZ_COMPONENTLIB_EXTRA_DSO_LIBS = mozcomps xpcom_compat >+ >+# Tell the linker where NSS is, if we're building crypto >+ifeq ($(OS_ARCH),Darwin) >+ifeq (,$(findstring crypto,$(MOZ_META_COMPONENTS))) >+MOZ_COMPONENTLIB_EXTRA_LIBS = $(foreach library, $(patsubst -l%, $(LIB_PREFIX)%$(DLL_SUFFIX), $(filter -l%, $(NSS_LIBS))), -dylib_file @executable_path/$(library):$(DIST)/bin/$(library)) >+endif >+endif >+endif >+ > # > # Include any personal overrides the user might think are needed. > # I'm not convinced that the following doesn't belong in modules/staticmod. Maybe modules/staticmod should be renamed and generalized? >--- /dev/null Wed May 28 20:21:56 2003 >+++ embedding/componentlib/.cvsignore Wed May 28 23:42:22 2003 >@@ -0,0 +1 @@ >+Makefile >--- /dev/null Wed May 28 20:21:56 2003 >+++ embedding/componentlib/Makefile.in Wed May 28 15:47:41 2003 >+ >+# We have to invoke ld directly so that force_data_segment.o can be linked >+# before dylib1.o >+MKSHLIB = $(LD) $(DSO_LDOPTS) -o $@ >+ >+# force it to be first >+SHLIB_LDSTARTFILE = force_data_segment.o -ldylib1.o >+SHLIB_LDSTARTFILE = -ldylib1.o Why is SHLIB_LDSTARTFILE here twice? Typo? The rest looks good.
Comment 4•21 years ago
|
||
Comment on attachment 124422 [details] [diff] [review] patch >Index: config/rules.mk >=================================================================== >+EXTRA_DSO_LDOPTS += -dynamiclib -install_name @executable_path/$(SHARED_LIBRARY) -compatibility_version 1 -current_version 1 Would it make sense to pull those verion numbers into separate make variables, so that we can rev them when we make changes to the lib that break backwards compatibility? >Index: xpfe/bootstrap/nsAppRunner.cpp >=================================================================== >+static nsStaticModuleInfo staticInfo[] = { >+ { >+ "meta component", >+ nsMetaModule_nsGetModule >+ } >+}; Can this be const? >+++ embedding/componentlib/Makefile.in Wed May 28 15:47:41 2003 >+# A limitation exists in the Mach-O format which causes "scattered" relocation >+# entries in shared libraries to be limited to a 24-bit address space (16 MB). >+# The high 8 bits of the address are dropped. This can cause problems in >+# a mozcomps build, particularly non-optimized builds. To avoid the problem, >+# we move the data segment before the code segment in the libraray. The data >+# segment is relatively small, and all of the relocations in question >+# (which are pointers to construction vtables) reside in the >+# (__DATA,__const) section. >+# >+# See also the 5th item in the TODO section of: >+# http://www.opensource.apple.com/darwinsource/10.2.6/cctools/ld/notes >+# (free APSL registration required) >+ >+ >+# XXX this causes radar bug 3268595 >+#EXTRA_DSO_LDOPTS += -exported_symbols_list exported_symbols Say what radar bug 3268595 is about. Is it the 24-bit limitation described above? >+++ embedding/componentlib/nsMetaModule.cpp.in Wed May 28 15:47:41 2003 >@@ -0,0 +1,120 @@ >+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- >+ * >+ * The contents of this file are subject to the Netscape Public >+ * License Version 1.1 (the "License"); you may not use this file Should this be NPL, or the MPL tri-license? Address comments, and sr=me
Attachment #124422 -
Flags: superreview?(sfraser) → superreview+
Assignee | ||
Comment 5•21 years ago
|
||
> Would it make sense to pull those verion numbers into separate make variables, > so that we can rev them when we make changes to the lib that break backwards > compatibility? Well, the only change to the library that could break backwards compatibility is changing the signature of nsMetaModule_nsGetModule. We've never actually used -current_version and -compatibility_version correctly for Gecko, I don't think. It would definitely make sense to use these version numbers correctly for the NSBrowserView framework. > Can this be const? Not without changing the definition of NSGetStaticModuleInfoFunc, since it has a non-const nsStaticModuleInfo out-pointer. > Say what radar bug 3268595 is about. Is it the 24-bit limitation described > above? No, it's not. This bug (which was since duped against radar 3126383, which means I can't follow it anymore) is about a situation where after prebinding is fixed for a binary, some C++ static initializers don't run anymore. I'll add a comment about this in the source. > Should this be NPL, or the MPL tri-license? I copied it from modules/staticmod/nsMetaModule.cpp.in, so I didn't figure it would be appropriate to change the license.
Comment 6•21 years ago
|
||
Comment on attachment 124422 [details] [diff] [review] patch r=ccarlen with the comment about SHLIB_LDSTARTFILE fixed.
Attachment #124422 -
Flags: review?(ccarlen) → review+
Assignee | ||
Comment 7•21 years ago
|
||
checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•