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.

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

Compiling with visibility=hidden #222

Closed
brendene opened this issue Jan 12, 2023 · 3 comments · Fixed by #226
Closed

Compiling with visibility=hidden #222

brendene opened this issue Jan 12, 2023 · 3 comments · Fixed by #226

Comments

@brendene
Copy link

Hello, thank you for developing this software.

When linking against quill built as a shared library with:

cmake_minimum_required(VERSION 3.21)

project(foo)

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED true)
set(CMAKE_CXX_VISIBILITY_PRESET hidden)
set(CMAKE_BUILD_TYPE RelWithDebInfo)

find_package(quill REQUIRED)

add_executable(logger Logger.x.cpp)
target_link_libraries(logger PRIVATE
  quill::quill
)

Using the example Logger.x.cpp:

//#pragma GCC visibility push(default)                                                                                                                                                                                                                                           
#include "quill/Quill.h"                                                                                                                                                                                                                                                         
//#pragma GCC visibility pop                                                                                                                                                                                                                                                     
                                                                                                                                                                                                                                                                                 
int main()                                                                                                                                                                                                                                                                       
{                                                                                                                                                                                                                                                                                
  quill::Config cfg;                                                                                                                                                                                                                                                             
  quill::configure(cfg);                                                                                                                                                                                                                                                         
  quill::start();                                                                                                                                                                                                                                                                
                                                                                                                                                                                                                                                                                 
  quill::Logger* logger = quill::get_logger();                                                                                                                                                                                                                                   
                                                                                                                                                                                                                                                                                 
  LOG_INFO(logger, "Welcome to Quill!");                                                                                                                                                                                                                                         
}                                         

The program will run without errors and output nothing. If you uncomment the pragmas then the program outputs the expected text.

Interestingly, adding (void)quill::detail::LogManagerSingleton::instance().log_manager().thread_context_collection().local_thread_context(); before the LOG_INFO causes the output to be printed.

@odygrd
Copy link
Owner

odygrd commented Jan 13, 2023

Thanks for reporting.

I have only tried to compile the library with the default visibility, is there a reason that the hidden visibility is needed ? My understanding is that visibility should only be applicable when building it as a shared library.

I will try to do some investigation at some point over the next few days

@brendene
Copy link
Author

Thanks for considering this.

I am building quill as a shared library and compile with hidden visibility to reduce binary sizes and to allow the compiler to optimize better. (https://gcc.gnu.org/wiki/Visibility)

On a side note, other high performance packages (ex apache arrow) also explicitly declare which symbols to export (https://issues.apache.org/jira/browse/ARROW-233)

@odygrd
Copy link
Owner

odygrd commented Jan 14, 2023

A fix for this should be in master soon, I have also added a CI build to avoid having it breaking again in the future.

Note that for maximum performance the recommendation is to compile Quill as a static library with LTO enabled and -march=native or something equivalent.

Thanks for reporting this and let me know if you have any other issues with the library

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants