Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ability create static library #85

Closed
wants to merge 1 commit into from
Closed

ability create static library #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 42 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,55 @@ set(SO_MAJOR 2)
set(SO_MINOR 0)
set(SO_PATCH 2)

add_definitions (
-DUTF8PROC_EXPORTS
)
# configure build type

set(_utf8proc_is_set_build_shared_libs TRUE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a weird variable name.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change to any other :)

if(DEFINED BUILD_SHARED_LIBS)
set(_utf8proc_is_set_build_shared_libs ${BUILD_SHARED_LIBS})
endif(DEFINED BUILD_SHARED_LIBS)

set(UTF8PROC_BUILD_AS_SHARED ${_utf8proc_is_set_build_shared_libs} CACHE BOOL "If present and true, this will cause utf8proc to be built shared unless the utf8proc was explicitly added as a static library.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does "explicitly added as a static library" mean?

Copy link
Author

@nicolaichuk nicolaichuk Sep 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my english is not good. Maybe you formulate a better this help phrase.

Copy link
Author

@nicolaichuk nicolaichuk Sep 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also. If you want. You can add support build as MODULE too.

in this case better rewrite something like this:

if(_utf8proc_is_set_build_shared_libs)
  set(UTF8PROC_BUILD_TYPE "SHARED" CACHE STRING "<some help message>")
else(_utf8proc_is_set_build_shared_libs)
  set(UTF8PROC_BUILD_TYPE "STATIC" CACHE STRING "<some help message>")
endif(_utf8proc_is_set_build_shared_libs)
set_property(CACHE UTF8PROC_BUILD_TYPE PROPERTY STRINGS "MODULE;SHARED;STATIC")

.....

if("${UTF8PROC_BUILD_TYPE}" STREQUAL "STATIC")
  list(APPEND _utf8proc_additional_defined 
    "-DUTF8PROC_BUILD_AS_LIB"
  )
else("${UTF8PROC_BUILD_TYPE}" STREQUAL "STATIC")
  list(APPEND _utf8proc_additional_defined 
    "-DUTF8PROC_EXPORTS"
  )
endif("${UTF8PROC_BUILD_TYPE}" STREQUAL "STATIC")

....

add_library (utf8proc ${UTF8PROC_BUILD_TYPE}
   utf8proc.c
   utf8proc.h
 )


# set additional defined

if (NOT MSVC)
if(NOT DEFINED _utf8proc_additional_defined)
set(_utf8proc_additional_defined )
endif(NOT DEFINED _utf8proc_additional_defined)

if(UTF8PROC_BUILD_AS_SHARED)
list(APPEND _utf8proc_additional_defined
"-DUTF8PROC_EXPORTS"
)
set(_utf8proc_build_type "SHARED")
else(UTF8PROC_BUILD_AS_SHARED)
list(APPEND _utf8proc_additional_defined
"-DUTF8PROC_BUILD_AS_LIB"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUILD_AS_STATIC seems more appropriate as the platform-independent antonym of "shared"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe UTF8PROC_STATIC_EXPORTS

)
set(_utf8proc_build_type "STATIC")
endif(UTF8PROC_BUILD_AS_SHARED)
list(REMOVE_DUPLICATES _utf8proc_additional_defined)
add_definitions("${_utf8proc_additional_defined}")

# other config

if(NOT MSVC)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O2 -std=c99 -pedantic -Wall")
endif ()
endif()

add_library (utf8proc
add_library (utf8proc ${_utf8proc_build_type}
utf8proc.c
utf8proc.h
)

if(MSVC)
get_target_property(_utf8proc_msvc_build_prop_val utf8proc COMPILE_FLAGS)
if(${_utf8proc_msvc_build_prop_val} STREQUAL "_utf8proc_msvc_build_prop_val-NOTFOUND")
set_target_properties(utf8proc PROPERTIES COMPILE_FLAGS "/Wall")
else(${_utf8proc_msvc_build_prop_val} STREQUAL "_utf8proc_msvc_build_prop_val-NOTFOUND")
set_target_properties(utf8proc PROPERTIES COMPILE_FLAGS "${_utf8proc_msvc_build_prop_val} /Wall")
endif(${_utf8proc_msvc_build_prop_val} STREQUAL "_utf8proc_msvc_build_prop_val-NOTFOUND")
endif()

set_target_properties (utf8proc PROPERTIES
POSITION_INDEPENDENT_CODE ON
VERSION "${SO_MAJOR}.${SO_MINOR}.${SO_PATCH}"
Expand Down
22 changes: 15 additions & 7 deletions utf8proc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,24 @@ typedef bool utf8proc_bool;
#endif
#include <limits.h>

#ifdef _WIN32
# ifdef UTF8PROC_EXPORTS
# define UTF8PROC_DLLEXPORT __declspec(dllexport)
#ifndef UTF8PROC_BUILD_AS_LIB
# ifdef _WIN32
# ifdef UTF8PROC_EXPORTS
# define UTF8PROC_DLLEXPORT __declspec(dllexport)
# else
# define UTF8PROC_DLLEXPORT __declspec(dllimport)
# endif
# elif defined(__GNUC__) && __GNUC__ >= 4
# define UTF8PROC_DLLEXPORT __attribute__ ((visibility("default")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are building a static library, it doesn't seem like the visibility attribute is relevant, since that seems to correspond to the ELF ABI for shared objects.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ifNdef UTF8PROC_BUILD_AS_LIB

in this place we are building shared library.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad; in that case the comment applies to the visibility("hidden") above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, this seems a bit superfluous, since the visibility is default by default.

# else
# define UTF8PROC_DLLEXPORT __declspec(dllimport)
# define UTF8PROC_DLLEXPORT
# endif
#elif __GNUC__ >= 4
# define UTF8PROC_DLLEXPORT __attribute__ ((visibility("default")))
#else
# define UTF8PROC_DLLEXPORT
# if defined(__GNUC__) && __GNUC__ >= 4
# define UTF8PROC_DLLEXPORT __attribute__ ((visibility("hidden")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we use hidden visibility here?

Copy link
Author

@nicolaichuk nicolaichuk Sep 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put simply, it hides most of the ELF symbols which would have previously (and unnecessarily) been public. This means:

  • It very substantially improves load times of your DSO (Dynamic Shared Object). For example, a huge C++ template-based library which was tested (the TnFOX Boost.Python bindings library) now loads in eight seconds rather than over six minutes!
  • It lets the optimiser produce better code. PLT indirections (when a function call or variable access must be looked up via the Global Offset Table such as in PIC code) can be completely avoided, thus substantially avoiding pipeline stalls on modern processors and thus much faster code. Furthermore when most of the symbols are bound locally, they can be safely elided (removed) completely through the entire DSO. This gives greater latitude especially to the inliner which no longer needs to keep an entry point around "just in case".
  • It reduces the size of your DSO by 5-20%. ELF's exported symbol table format is quite a space hog, giving the complete mangled symbol name which with heavy template usage can average around 1000 bytes. C++ templates spew out a huge amount of symbols and a typical C++ library can easily surpass 30,000 symbols which is around 5-6Mb! Therefore if you cut out the 60-80% of unnecessary symbols, your DSO can be megabytes smaller!
  • Much lower chance of symbol collision. The old woe of two libraries internally using the same symbol for different things is finally behind us with this patch. Hallelujah!

All new code should have this support from the beginning!

read more https://gcc.gnu.org/wiki/Visibility

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why you would want to hide symbols that you are not exporting. We are already doing this by declaring internal functions as static (which works because all of our code is in a single .c file). But UTF8PROC_DLLEXPORT is used for exported functions. Surely we don't want to hide those symbols?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this code is for when UTF8PROC_BUILD_AS_LIB is defined, so we are not even building a shared library and the visibility attribute is ignored, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole point of these symbols is that they are the public API of this library. Making them hidden makes little sense. End users can decide whether to ask to linker to hide them or not when they build this into their libraries and applications.

Copy link
Author

@nicolaichuk nicolaichuk Sep 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When i build static library - this meen that i want use this library for internal purposes only. And it hide for external user.
When i want external uses - i will build SHARED or MODULE version.

I think that using visibility("default") for all symbols - is legacy gcc practices.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When i build static library - this meen that i want use this library for internal purposes only. And it hide for external user.

It just means that this code is not intended to be used as a shared library (by itself). People would very well decide to use -Wl,--whole-archive and re-export those symbols. Visibility is an API concern and whether or not this library is build shared does not affect what the library's public API is.

Copy link
Author

@nicolaichuk nicolaichuk Sep 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe.
If you want you can change this to visibility("default").
It is not principal thing for me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, just leave that part as-is.

# else
# define UTF8PROC_DLLEXPORT
# endif
#endif

#ifdef __cplusplus
Expand Down