Hello, On 19.07.2012 03:11, Greg Banks wrote:
On Wed, Jul 18, 2012, at 10:46 PM, Дилян Палаузов wrote:On 16.07.2012 02:08, Greg Banks wrote:So, remind me again what actual value we're getting from this -fvisibility=hidden stuff again?Initially, libcyrus_sieve had a lot of exported symbols generated by bison and flex. When libcyrus_sieve was loaded by the dynamic linker the list of exported symbols (section .dynsyms) was long, so finding the needed symbol by the dynamic linker was supposed to take long time.Well, maybe. On Linux at least, the linker puts a hashtable index to the exported symbols into a section called .gnu.hash and the runtime linker uses that to speed up searching for symbols. So lookup is going to be something closer to O(1) than O(N). So I'm unconvinced by an argument from runtime link performance.
There is always a table making the search faster than O(n), the gnu linker's table (--hash=style=gnu) is better than the standard table (--hash-table=sysv), but still, less symbols means smaller table. The arguments from run time link performance are not fucking strong, but the resulting executable file/library are smaller.
There is an argument to be made from untidiness.
gnb@gnb-desktop 2069 nm -D --defined-only
./sieve/.libs/libcyrus_sieve.so | awk '{h[$2]++}END{for (i in h) print
h[i],i}'
3 A
10 B
3 D
115 T
That's a lot of exported symbols for such a small library :(
But you could fix that without advanced non-portable linker trickery, by
(for example) moving chunks of sieve/tree.c into sieve/sieve.y, or
#include'ing one into the other or vice versa.
The most T-symbols are generated by flex/bison and this cannot be tricked. Flex can be instructed not to generate unnecessary functions with %option noyyset_in or alike, however flex on ci.cyrus-imapd.org does not support these options. So the functions are there.
Moving functions from tree.c to sieve.y is marginal compared to the amount of functions created by bison/flex.
The non-portable trickery does not harm. It is supported by GCC and Clang (according to http://clang-developers.42468.n3.nabble.com/Does-clang-support-attribute-visibility-quot-default-quot-td3944043.html ). Anyway, with -fvisibility=hidden approximately 60-70 symbols are internalized/hidden, that would be otherwise exported in the .dynsyms section.
Със здраве Дилян
Annotating in libcyrus_sieve (and the other libraries) the functions/symbols with EXPORTED, that are needed outside the library, and compiling with -fvisibility=hidden, keeps the list of exported symbols short, so the dynamic linker can load the library faster. On the other side the resulting library is smaller. In addition, I think this makes things easier to maintain, as it is clear, if a function is used outside the library (EXPORTED), only within the library (HIDDEN), or only within the source file, it is defined (static)I think -fvisibility=hidden is an excellent mechanism for folks maintaining real actual libraries with defined and documented interfaces at both the source and ABI levels, to enforce that users of their libraries use only the supported interfaces. Cyrus really isn't one of those. We have no documentation, no ABI guarantee, no versioning mechanism. Worse, we have a lot of truly horrible tricks which rely on traditional linktime semantics, fatal() being the obvious example. Those can and should be fixed, but right now they're pretty broken.Compiling different Automake targets with different CPPFLAGS/CFLAGS creates .o files with very long names, when the non standarf AM_CFLAGS are used. The reason for the long name is, that Automake assumes, that a file can be compiled once with the non-standard (not Makefile.am-wide) CFLAGS, and once with AM_CFLAGS, so Automake reserves its right to create different .o files from the same .c file. This could be avoided, if -fvisibility=hidden is used not only for libraries but also for executables and is put in AM_CFLAGS.Sure, if it's one place it should be in as many places as possible.
<<attachment: dilyan_palauzov.vcf>>
