Opened 17 years ago
Last modified 16 years ago
#1810 closed enhancement
[PATCH] Adds --with-hide-internal-symbols options to configure — at Version 2
Reported by: | Even Rouault | Owned by: | Mateusz Łoskot |
---|---|---|---|
Priority: | normal | Milestone: | 1.5.0 |
Component: | ConfigBuild | Version: | svn-trunk |
Severity: | normal | Keywords: | gcc |
Cc: | warmerdam, Daniel Morissette, Mateusz Łoskot |
Description (last modified by )
Quoting a recent discussion raised on gdal devel list :
Pre-scriptum : it may be usefull to read http://gcc.gnu.org/wiki/Visibility to understand some points of this discussion.
Reducing the number of symbols has a lot of benefits as stated in the above link :
- speed up loading of shared objects
- better opportunities for code optimizations
- reduce the size of SO.
- much lower chance of symbol collisions.
I've looked at the number of exported symbols by libgdal.so. It reaches the impressive number of 10 000 on my Linux system ! You can test on your own system with : nm -C -D libgdal.so | wc -l Then, I've grabbed FWTools136.exe and looked at gdal_fw.so. It "only" exports 3 549 symbols. The reason is that on Windows, you have to explicitely declare that a symbol is visible from outside the library. This is done in GDAL with the CPL_DLL macro that expands to
__declspec(dllexport)
for MSC.
However with GCC, symbols are implicitely considered as public/exported unless otherwise specified. Since GCC 4.0, we have a way to change that behaviour through the -fvisibility=hidden flag. Thus we can define this flag and modify the CPL_DLL declaration in cpl_port.h like hereafter to explicitely export public symbols:
#ifndef CPL_DLL #if defined(_MSC_VER) && !defined(CPL_DISABLE_DLL) # define CPL_DLL __declspec(dllexport) #else # if defined(__GNUC__) && __GNUC__ >= 4 # define CPL_DLL __attribute__ ((visibility("default"))) # else # define CPL_DLL # endif #endif #endif
I give it a try on my system. This required only another change in shapefil.h that doesn't use CPL_DLL but SHPAPI_CALL that is equivalent.
#ifndef SHPAPI_CALL # if defined(__GNUC__) && __GNUC__ >= 4 # define SHPAPI_CALL __attribute__ ((visibility("default"))) # define SHPAPI_CALL1(x) __attribute__ ((visibility("default"))) x # else # define SHPAPI_CALL # endif #endif
Result : only 3 442 symbols ! Roughly like on Windows :-)
Another great benefit is that it should prevent subtle and annoying problems when linking an executable with a GDAL library build with internal libtiff/libpng/libjpeg/etc... and one of these libraries.Now most of them have disappeared form libgdal.so. This may be becoming necessary with new libtiff 4.0 that has become the internal GDAL libtiff, whereas most *nix systems/distributios still uses 3.8.2. People will probably want to use BigTIFF capabilities without messing up their system libtiff. Mixing symbols from both versions is definitely not a good idea as there has been ABI changes. The case for internal libgeotiff is a bit different since it uses CPL_DLL too in its header. So, symbols for internal libgeotiff are still exported. We should fine a way to distinguish the case when it's built in GDAL tree from when it's build standalone.
So, to sum up, if we want to integrate this in GDAL, we have to :
- change cpl_port.h and shapefil.h
- add a test for the support of -fvisibility=hidden in configure
Any thoughts on all of this ?
(I also tried to use -fvisibility-inlines-hidden but it fails on my system since there's a bug in GCC 4.1 (apparently fixed in GCC 4.2). Anyway, it should not make a large difference since GDAL code is not using lot of inline functions.)
Change History (3)
comment:1 by , 17 years ago
by , 17 years ago
Attachment: | gdal_svn_trunk_hide_symbols.patch added |
---|
comment:2 by , 17 years ago
Cc: | added |
---|---|
Component: | GDAL_Raster → ConfigBuild |
Description: | modified (diff) |
Keywords: | gcc added |
Owner: | changed from | to
Patch applied (r12110), the configure changes look fine. I did a few tweaks to text to keep the option to one line and grouped the end-of-configure report a bit differently. I also upstreamed the shapefil.h change into shapelib so the next time we pull that down we won't lose it.
I don't have gcc4 on my system so I can't easily check this. I see the buildbot.osgeo.org uses gcc 4.0.2, so I'm reassigning this to Mateusz to modify buildbot to use this configure option.
I'm attaching a patch that implements this.
Please review carefully as I'm a total beginner with autoconf stuff.