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)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1438688

People

(Reporter: benjamin, Unassigned)

References

Details

Attachments

(2 files)

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
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.
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.
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.
Attached patch Patch (deleted) — Splinter Review
I should probably turn off warnings for this generated file too. It spews a ton of compiler warnings.
Assignee: benjamin → khuey
Status: NEW → ASSIGNED
Attachment #671988 - Flags: review?(benjamin)
This adds approximately 37k relocations on Windows (out of a total 450k).
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 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?
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)
(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
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.

Attachment

General

Created:
Updated:
Size: