Opened 5 years ago

Last modified 4 years ago

#3764 accepted enhancement

Add visibility support

Reported by: fdupont Owned by: fdupont
Priority: low Milestone: Outstanding Tasks
Component: build system Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

The reference are the gcc manual -fvisibilty entry and https://gcc.gnu.org/wiki/Visibility.
I propose to add visibility=default attributes to all classes, functions, etc, which should be exported in a WIN32 compatible way, and to add a new configuration option for -fvisibility=hidden but with the current no -fvisibility for default.
Cons:

  • we'll have to add a config with hidden visibility in the test platform.

Pros:

  • better handling of must be internal things (i.e., improve code quality)
  • faster and smaller binaries (for SoftHSMv2, perhaps an extreme example, the minimum binary size reduction was 40%)
  • compatibility with platforms where default visibility is hidden (e.g., WIN32).

BTW I have a BIND 10 ticket doing the hard part and some examples (SoftHSMv2, log4cplus which BTW is a dependency) so it should be easy.

Subtickets

Change History (10)

comment:1 Changed 5 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:2 Changed 5 years ago by fdupont

I did significant progress even it is not yet finished. 3 points to discuss:

  • addint #include <config.h> in src/lib/log/compiler/message.c makes the VERSION variable to conflit (easy to fix, I noted this because it can be addressed independently of this ticket).
  • at the noticeable exception of src/lib/hooks/tests/Makefile.am test Makefiles which build DSOs incorrectly set the CXXFLAGS to KEA_CXXFLAGS. There are 3 ways to solve this: don't inherit KEA_CXXFLAGS, introduce an alternative CXXFLAGS or modify the DSO source codes. IMHO the simplest so the best is the first and when it is the solution used for the hooks lib there is no good reason to do something else for other similar cases (same note than the preceding point).
  • the last point is internal to this ticket: I can write a short doc explaining how to add visibility handling in the code. Where should I put it? (Wiki, doc/design, here, etc)

comment:3 Changed 5 years ago by fdupont

Changed my mind about the second point (hooks/DSOs): to compile executables (vs shared libs) with -fvisibility is at least useless so a new CXXFLAGS is needed. Anyway all Makefiles for DSOs should use the same flags (but it is a very low priority item).

Last edited 5 years ago by fdupont (previous) (diff)

comment:4 Changed 5 years ago by fdupont

On Fedora 20, -fvisibility=hidden gives ~2% downsize but > 20% on stripped objects (BTW it does not make sense to not strip shared libraries when space matters as the size ratio is 10 :-).
Should try one day with -fvisibility-inlines-hidden too.

comment:5 Changed 4 years ago by tmark

  • Component changed from Unclassified to build system

comment:6 Changed 4 years ago by fdupont

Found the origin of the data problem. Begin the doc in the wiki. In conclusion: I shall resume work on this soon.

comment:7 Changed 4 years ago by tomek

  • Milestone changed from Kea-proposed to DHCP Outstanding Tasks

Discussed on 2015-05-20 Kea call. While this would be very useful to have, unfortunately we do not have time to spend on visibility and decided to move to DHCP Outstanding.

comment:8 follow-up: Changed 4 years ago by tomek

As for the last bullet in comment:2, I think the best place would be in the Developer's Guide (i.e. somewhere in doxygen comments or in a .dox file).

comment:9 in reply to: ↑ 8 Changed 4 years ago by fdupont

Replying to tomek:

As for the last bullet in comment:2, I think the best place would be in the Developer's Guide (i.e. somewhere in doxygen comments or in a .dox file).

=> It is in a wiki page today... We should reconsider this when this work will resume.

Note I'll wait another occasion to finish this. There is no unknown issue (this doesn't mean there is none, in particular how to get the best occasion to merge something which changes all the files used in shared libraries :-).
I'd like to stall this ticket but there is no such status so I'll just keep it.

comment:10 Changed 4 years ago by tomek

  • Milestone changed from DHCP Outstanding Tasks to Outstanding Tasks

Milestone renamed

Note: See TracTickets for help on using tickets.