Closed
Bug 685241
Opened 13 years ago
Closed 7 years ago
Compile the core XPTs into libxul instead of reading them from disk
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 1438688
People
(Reporter: benjamin, Unassigned)
References
Details
Attachments
(2 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
We can and should compile the core XPT files into libxul instead of reading them from disk. The strategy is:
* if LIBXUL_LIBRARY is set, copy the xpts to dist/libxul-xpt instead of dist/xpt (probably some makefile cleanup necessary).
* In toolkit/library run a python tool to read the XPT files (not the sources) and output the information in XPTHeader and substructure format (may require some C++ magic to deal with the variable-length structures that may be part of XPT, still not sure exactly how that works).
* Import the XPTHeader directly using xptiInterfaceInfoManager::RegisterXPTHeader
Comment 1•13 years ago
|
||
Is there a reason you'd want to produce XPT files at all, instead of just adding a mode to typelib.py to generate the source files directly? If you generate the typelib structure using the classes from xpt.py, that's exactly what you'd get if you used xpt.py to read an XPT file from disk.
Reporter | ||
Comment 2•13 years ago
|
||
We need a fully linked version of the XPTs to avoid repeating structure data for the base interfaces and types. Unless we're going to implement all IDL compiling at once, but I don't think that's in scope for this.
Comment 3•13 years ago
|
||
We could just treat them as externs in the generated code, like:
#include "nsISupports_typelib.h"
XPTInfo nsIFoo_typelib = {
nsISupports_00000000_0000_0000_c000_000000000046, // parent
// ...
};
Although I'm not totally sure how the data structures work, to be honest. Maybe we can worry about that in a followup. Just seems silly to generate all the XPT files and then throw them away.
I should probably turn off warnings for this generated file too. It spews a ton of compiler warnings.
Blocks: 799658
This adds approximately 37k relocations on Windows (out of a total 450k).
Comment 6•12 years ago
|
||
Here's a proof-of-concept patch that cuts down the size of the .o file by
about 10% on x86-64. I think it'd be relatively more on 32-bit platforms
because the other, pointer-dependent data structures would shrink in size,
while the parameter descriptor structures would remain the same size.
The same idea can obviously be applied to the parameter arglist
descriptors. The only wrinkle is dealing with array types and things like
that, which I hadn't managed to work out in the short time I gave myself
for playing with this bug. There's obviously more variety among parameter
lists, so it might not be very helpful.
Comment 7•12 years ago
|
||
Comment on attachment 671988 [details] [diff] [review]
Patch
Review of attachment 671988 [details] [diff] [review]:
-----------------------------------------------------------------
::: toolkit/library/typelib/Makefile.in
@@ +25,5 @@
> +$(CURDIR)/typelib_gen.c: $(CURDIR)/libxul.xpt
> + $(PYTHON) $(LIBXUL_DIST)/sdk/bin/xpt.py embed $@ $^
> +
> +$(CURDIR)/libxul.xpt: $(DEPTH)/staticlib/xpt
> + $(XPIDL_LINK) $@ $(DEPTH)/staticlib/xpt/*.xpt
That's going to blatantly fail to do the right thing for Metro builds and Firefox built as a xulrunner application.
That's also most likely going to break comm-central.
I don't understand. Can you be more specific?
Reporter | ||
Comment 9•12 years ago
|
||
Comment on attachment 671988 [details] [diff] [review]
Patch
Sorry for the lag. khuey is going to separate the LIBXUL_LIBRARY changes from the guts of this patch so that it's easier to review.
Attachment #671988 -
Flags: review?(benjamin)
Comment 10•12 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #8)
> I don't understand. Can you be more specific?
I don't know if this is what Mike means, but you can't do the obvious:
cd toolkit/library/typelib/ && make typelib_gen.c
Why do you think you need $(CURDIR) in there? Why not just typelib_gen.c?
Assignee: khuey → nobody
Status: ASSIGNED → NEW
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•