685241 - Compile the core XPTs into libxul instead of reading them from disk

Status: RESOLVED DUPLICATE of bug 1438688

(Core :: XPCOM, defect)

Description

[Benjamin Smedberg]

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

[(not currently active) Ted Mielczarek]

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.

Comment 2

[Benjamin Smedberg]

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

[(not currently active) Ted Mielczarek]

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.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

I should probably turn off warnings for this generated file too.  It spews a ton of compiler warnings.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This adds approximately 37k relocations on Windows (out of a total 450k).

Comment 6

[Nathan Froyd [:froydnj]]

Attachment: uniquify result type descriptors

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

[Mike Hommey [:glandium]]

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.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I don't understand.  Can you be more specific?

Comment 9

[Benjamin Smedberg]

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.

Comment 10

[Nathan Froyd [:froydnj]]

(In reply to Kyle Huey [:khuey] ([e-mail]) 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?