Forked from bug 313207 - provide a table-driven QI mechanism which will reduce codesize and probably also increase QI performance due to processor cache locality.

Benjamin, are you diving into this? I had some thoughts, which might wait for JS2 work to get implemented. One is to make single-threaded isupports QI maps be self-organizing. Another is negative caching, to handle the frequent miss testing problem. I'm happy to do this as a later step on top of table-driven QI at least for the simple cases, but I wanted to mention it now in case it seems worth doing now, or working together on the design more. /be

Previous attempt is in bug 23737.

This uses a table-driven QI mechanism where a IID->offset table is statically constructed and passed to NS_TableDrivenQI for ordinary cases. Inheriting and aggregated versions are handled in the tail of the QueryInterface method. This proves a significant codesize win on all platforms. The performance tests I was able to do are a little bit more ambiguous: linux was a definite win (close to 8% speedup on a contrived looping testcase), while win had a 0.5% slowdown (same contrived testcase). I think that the numbers are close enough to good that I'd like to try and land this and see what the tinderbox perf numbers say. This patch is just the xpcom/ bits... the rest of my tree is littered with update NS_DEFINE_STATIC_IID_ACCESSOR fixups for pseudo-interfaces which I don't intend to attach unless you want to see them for some reason.

Beauty. I don't think synthetic loop torture-benchmarks are meaningful. I'm all in favor of getting this into some testerboxes, or just into the trunk, and seeing what Tp, Txul, etc. are. /be

Does placing NS_DEFINE_STATIC_IID_ACCESSOR in header files really work properly with all supported compilers+linkers?

It works on MSVC.net2003/linux-gcc4.0/mac-gcc3.3

Do those linkers coalesce duplicated nsID instances? What about vc6sp5? What about GCC 2.95.2 (i.e., gcc w/o NS_HIDDEN)?

Absolutely: C++ templates wouldn't make any sense unless they used weak/gnu-linkonce to coalesce symbols (the templatized hashtables would be enourmous!)

In theory one could use some sort of hashing to avoid a loop. For example, if you know that there is some N-bit subfield of the IIDs for a class such that the subfield of each IID gives a unique value, then you can implement QI by extracting the bitfield, using it to index a table, and comparing the given IID with the IID in the table; if the IIDs don't match then you know the interface is not supported. Unfortunately constructing such tables would require some serious magic.

gperf?

Computing what the subfield and IID table should be, given a list of IIDs, is easy; brute force would suffice. The magic part would be integrating that algorithm into the build.

I suppose this doesn't work, does it? #define NS_DEFINE_STATIC_IID_ACCESSOR(the_interface, the_iid) \ NS_SPECIALIZE_TEMPLATE struct COMTypeInfo<the_interface> { \ static const nsIID kIID NS_HIDDEN; \ }; \ const nsIID COMTypeInfo<the_interface>::kIID NS_HIDDEN = the_iid; #define NS_INTERFACE_TABLE_ENTRY(_class, _interface) \ { &COMTypeInfo<_interface>::kIID, \ etc. etc.

No, the NS_SPECIALIZE_TEMPLATE stuff uses strong symbols on windows instead of weak symbols, so you end up with multiply-defined-symbol errors :-(

Comment on attachment 200781 [details] [diff] [review] Static table-driven queryinterface, rev. 1 This looks fine. I have a slight preference for landing the FASTCALL changes first, to isolate strangeness and perf effects. I trust we have build-size and perf numbers for all of the big 3, at least? r=shaver

The fastcall bits have landed. I'm going to land the NS_DECLARE/NS_DEFINE macro changes tomorrow morning with a closed tree... I'll change the templated NS_GET_IID on Monday morning and do the table-driven QI switch last, Monday afternoon or Tuesday.

Part 2, which changes xpidl and the pseudo-interface declarations in the tree, has been checked in.

Part 3 was checked in and then backed out, the old mac tboxen with xcode 1.1 were broken by the template changes, as well as MSVC6 which was having problems with ambiguous name resolution of multiply-inherited interfaces. I was able to fix MSVC6 pretty easily by making NS_GET_IID use a ::Interface, but the xcode stuff will have to wait until the tinderboxen are upgraded (we've already officially stopped supporting building with xcode1.1). Bug 299214 covers the tinderbox upgrade.

Part 3 checked in and backed out again, broke gcc2.9x and MSVC6 (different error this time, I think it has to do with "typedef class nsIFrame nsIBox;" in nsIFrame.h.

Could it be considered to support table-driven QI on known "good" compilers ? (I'm thinking of VS 2005)

Revision 3.2 of xpcom/glue/pldhash.h, attributed to this bug, changed a generated file, pldhash.h, without changing the file from which it was generated, js/src/jsdhash.h.

Fixed on trunk. I'll file followups to make in-tree code that currently uses the old-style interface maps use the new table-driven ones.

I wonder if this broke something on BeOS (gcc 2.95.3, ld 2.15). Short version (adding a longer output as patch): When linking libtoolkitcomps.so ../../../dist/lib/liburlclassifier_s.a(nsUrlClassifierDBService.o)(.nsUrlClassifierDBService::gnu.linkonce.t.GetIID(void)+0x12): In function `nsUrlClassifierDBService::GetIID(void)': /mozdev/dbg-ffox/toolkit/components/url-classifier/src/../../../../dist/include/xpcom/nsCOMPtr.h:467: undefined reference to `nsUrlClassifierDBService::COMTypeInfo<int>::kIID'

gcc2.9x does not support the proper templated static members for this patch and is unsupported.

So basically the BeOS-platform is no longer supported at all?

well, be-os has never been officially supported. :-)

Well it was a port under Mozilla. (Just want to gather facts on what do next.) I guess I should read up on the new minimum build requirements. Is there any other info I can find on this?

even with the smileyface that sounded pretty mean. sorry. I would think that for BeOS, you would try out gcc 3.4.3 or whatever the latest gcc for BeOS is. I quickly read over the release notes and it doesn't sound like any of the bugs will block building ff.

Benjamin checked in the patch in three parts which aren't attached here as separate diffs. I already reverted part three while debugging another bug, so here it is in case anyone else finds it useful for testing.

I've heard there is a version of BeOS using gcc3.x that we could still compile on. But yes, the trunk code is not going to work on the older BeOS. I don't know that we have a good place to document this other than perhaps http://developer.mozilla.org/en/docs/Linux_Build_Prerequisites#Build_Tools

BeOS has newer gcc's but they only work for C not C++ due to the ABI changes in gcc3. The kernel and all libs are in the old ABI. Therefore we can no longer build trunk or supply Firefox for the existing BeOS. Haiku which is an upcoming opensource version will support gcc4, but I don't think it would be anywhere near stable to compile Firefox on, and it's netstack is not even finished. So if gcc2.95.3 support is dropped it's pretty much end of the line for BeOS altogether with probably a new beginning 1/2-1year from now with Haiku. (http://haiku-os.org/)

BeOS successor Zeta OS promised gcc4-based version somewhere at 2.0, but now it is at 1.21, and gcc4-compiled version exists only on main developer harddisk. But there is one half-optimistic detail. Zeta OS maintainer told once that they have something like ABI-wrapper for Zeta already, which allows to work gcc4-compiled C++/GUI programs to work on Zeta 1.21. I'm wondering if he can publish those wrappers, aswell gcc4 itself, to use it for all BeOS version, but have deep doubts there, as he is for sure more interested in selling Zeta OS, not in keeping obsolete versions of OS alive.

(In reply to comment #24) > gcc2.9x does not support the proper templated static members for this patch and > is unsupported. > Bug 361542 opened to document BeOS bustage and a place to document any possible workaround.

It appears from https://bugzilla.mozilla.org/show_bug.cgi?id=361542#c11 that BeOS build wasn't fundamentally broken due gcc 2.953 legacy - taht old gcc is simply "too strict":) and bustage can be fixed with explicit correct definition.

Plugins and extensions that use the NS_IMPL_ISUPPORTS macros (and build in the Mozilla source tree) may have a hard time with this: Unless they can find a way to weakly link in NS_TableDrivenQI(), they'll have to unroll their NS_IMPL_ISUPPORTS macros. Have you considered maintaining versions of the NS_IMPL_ISUPPORTS macros that don't use table-driven QI? (I've already worked around the problem in my own plugin (the MRJ Plugin JEP component of the Java Embedding Plugin for Mac OS X), and I don't actually know if any other plugins or extensions use the NS_IMPL_ISUPPORTS macros. So the problem may not be urgent. In fact I neither unrolled my NS_IMPL_ISUPPORTS macros nor used weak linking: OS X supports something called the "two-level namespace", which allows multiple definitions of symbols, but which also does a very good job of choosing the "right" one.)

Steven, I'm not sure what weak linkage has to do with this. NS_TableDrivenQI is provided by the XPCOM glue static library and should be linked to there. It also happens to be exported from libxpcom_core.dylib for use by internal-linkage code, but code like JEP really shouldn't be using internal linkage.

The MRJCarbon plugin (and my MRJ Plugin JEP successor to it) has used the NS_IMPL_ISUPPORTS for many years. It used to be you could do this without linking in any browser code. Now, if you want to keep using NS_IMPL_ISUPPORTS, you have to link in NS_TableDrivenQI().

That's how the glue is designed, and it's required for all sorts of other uses, like nsCOMPtr. Link and be happy!

> Link and be happy! My whole point is that linking gives you lots of reasons to be unhappy.

Was that your point? You haven't said *why* it makes you unhappy.

> You haven't said *why* it makes you unhappy. Yes I did. In comment #35. But to restate my point succinctly: Having to link in NS_TableDrivenQI() makes things harder for writers of plugins and extensions, because they have to worry about that symbol being multiply defined -- once in the plugin/extension, and another time in the browser. As I also said in comment #35, I've found a workaround for the problem, so _I'm_ not particularly unhappy. I just wanted to point out something that you may have overlooked (and apparently did overlook), which might impact the authors of other plugins and extensions.

In the light of a new day, I realize I made a dumb mistake. I mixed up the following: 1) A host program loading a shared library when both define the same (public, external) symbol -- which can cause trouble. 2) A host program loading a shared library that staticly links to a (public) symbol defined in the host. This means the aggregate will have two copies of whatever the symbol points to (and other dependent symbols it may drag in). But this won't be a problem if the symbols point to code (i.e. if the dependent symbols don't include any global variables). A shared library that uses NS_IMPL_ISUPPORTS (and staticly links in NS_TableDrivenQI()) fits case #2. I withdraw all my objections. Sorry for the confusion :-(

> A host program loading a shared library "Loading" should have been "dynamically loading".