Improve visibility annotations for the library

[Olly Betts]

See ​http://gcc.gnu.org/wiki/Visibility

This should make the shared library much smaller, and a little faster!

[Richard Boulton]

It would be lovely to do this for 1.0. Possibly not important enough to worry about right now, though.

[Olly Betts]

I've put this off previously as visibility annotations are only useful for GCC >= 4.0. However, such versions are now deployed by many linux distros, so I think the time has come for this, and it's not really something we can retrofit into the 1.0 releases. It shouldn't be a lot of work, but it would be good to do it now so any issues have a chance to be shaken out before release.

So I've just made a first pass. The library builds, as do simple utilities like "delve". Things like "quartzcheck" which hook into quartz don't build, and neither does the test harness, so it needs some more work still.

Currently (on x86_64) the stripped libxapian.so is 8% smaller (unstripped is 2.6% smaller), while the number of external symbols drops from 2851 to 508.

[Olly Betts]

Hmm, what a mess!

The test harness uses a few bits of the library internals, which is a bit unclean but we can expose those for now at least.

Doing that, apitest builds and passes, which is a promising sign.

But quartztest, btreetest, and internaltest don't stand a chance without exposing shedloads of library internals.

Looking at quartztest, much of it can either just be removed or turned into API level tests where such tests don't already exist.

Not sure what to do about the rest. Unit tests are a good idea, so deleting them isn't really a good option (btreetest and quartztest are less important now flint is the default, but we ought to at least consider a flint btree-level test).

We could try to just link against the object files which are actually needed, but then we also need utility functions, debug functions, etc and this seems likely to be a maintenance headache.

Passing "-static" when linking these problematic binaries is a fix which works (though it's not ideal either). This only links libxapian statically at least - libstdc++ and the shared library aren't affected. Also, we only need to do this for GCC >= 4.0. If we go this route, I think we should merge all the affected tests into a single binary (or fewer binaries at least).

Any other ideas?

[Richard Boulton]

Possibly not a helpful comment - but don't the utilities in xapian-core/bin/ also need to link to various internals? It would be nice if we could move sufficient amounts of their functionality into the API, so the utilties could be implemented with just the public API (with the side benefit that things like the python bindings could perform a check of a database without needing to exec a sub-process). However, that's not going to happen in the near future.

The -static approach seems the only way forward to me: if the symbols aren't exported by the library, there's no way that utilities are going to be able to link to it, however clever we are. So, the question is whether it's worth doing given that cost.

This isn't a particularly urgent fix, in my mind; though 8% would be a nice size improvement, I've not come across anywhere recently where size is a problem (though if someone wanted to use Xapian in an embedded system it might become one). The attraction of doing this is more to keep the API neat, and reduce the probability of symbol collisions. Perhaps we should defer this to 1.1, and try and move tests into fewer binaries and work on the utilities in the meantime.

[Olly Betts]

The utilities in xapian-core/bin do indeed, but the API we need to expose there is fairly limited, so for now I've just marked those as visible (which isn't actually worse than just using them because they happen to be visible if you include the right header!) As you say, longer term it would be better to make those implementable through the API. In particular, being able to merge databases would be handy when writing an indexer.

Given I've got this far, I'm inclined to do a bit more work and commit this, even if it means marking all the classes and functions used by the testsuite as visible (at least some things will be hidden, which would be a step forward).

In the longer term, a coherent approach to allowing unit testing of things like the serialisation functions would be useful though...

[Richard Boulton]

As you say, hiding even some bits of the internals is better than nothing at all, and getting the infrastructure in place to remove more when we are able to is also good. I hadn't thought of doing a partial implementation like that.

[Olly Betts]

I've just committed changes to implement this.

For now, all the internals required by stuff in xapian-core/bin and xapian-core/tests is simply marked as visible. We can look at removing the need for some of these dependencies (e.g. by adding API methods to allow databases to be merged or checked) and think about a strategy to allow sane unit testing.

I've also not worried about marking internal members of visible classes as non-visible - the gain from that is likely to be fairly small, so that can wait until later too I think.

So I'm retitling this bug, and removing the blockage on 1.0.

[Olly Betts]

Flag for reassessing during the 1.1.x series.