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

upgrade prometheus-cpp to v1.0.1 #5279

Merged
merged 2 commits into from
Jul 3, 2022
Merged

upgrade prometheus-cpp to v1.0.1 #5279

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0743ae
upgrade prometheus-cpp to v1.0.1
YangKeao Jul 3, 2022
aedda8a
remove condition to link with libatomic
YangKeao Jul 3, 2022
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
2 changes: 1 addition & 1 deletion contrib/prometheus-cpp
Open in desktop
Submodule prometheus-cpp updated 126 files
6 changes: 6 additions & 0 deletions contrib/prometheus-cpp-cmake/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ target_link_libraries(core
$<$<AND:$<BOOL:UNIX>,$<NOT:$<BOOL:APPLE>>>:rt>
)

if(HAVE_CXX_LIBATOMIC)
Copy link
Contributor

Choose a reason for hiding this comment

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

may I ask which symbol is utilized here?

Copy link
Contributor

@SchrodingerZhu SchrodingerZhu Jul 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

libatomic is a sublib of libgcc series to dispatch atomic operations. clang however should generate instrinction directly for most atomic operations as long as the hardware feature flags are set. We do not set -mcx16 yet for tiflash, but that is something generally available for x86-64-v2 target, which is within the assumption of tiflash. So, if that is the problem, I would prefer adding -mcx16 on x86-64 platform instead.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
  # First check if atomics work without the library.
  check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITHOUT_LIB)
  # If not, check if the library exists, and atomics work with it.
  if(NOT HAVE_CXX_ATOMICS_WITHOUT_LIB)
    check_library_exists(atomic __atomic_load_8 "" HAVE_CXX_LIBATOMIC)
    if(HAVE_CXX_LIBATOMIC)
      list(APPEND CMAKE_REQUIRED_LIBRARIES "atomic")
      check_working_cxx_atomics(HAVE_CXX_ATOMICS_WITH_LIB)
      if(NOT HAVE_CXX_ATOMICS_WITH_LIB)
        message(FATAL_ERROR "Host compiler must support 64-bit std::atomic!")
      endif()
    else()
      message(FATAL_ERROR "Host compiler appears to require libatomic for 64-bit operations, but cannot find it.")
    endif()
  endif()
endif()

checking into the implementation, this seems to be a compatible flag for special architectures without even 8-byte atomic load. I think we can skip the check here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just (carelessly) copy these CMakeLists from the upstream. Nothing will set this variable so it's fine to remove this condition.

The upstream has a another CMakeList, which checked whether the compiler can compile without libatomic. If not, the HAVE_CXX_LIBATOMIC will be set. See jupp0r/prometheus-cpp@d0e1056

I've removed these lines in the latest commit

# the exported library config must use libatomic unconditionally
# (the HAVE_CXX_LIBATOMIC variable should not leak into the target config)
target_link_libraries(core PUBLIC atomic)
endif()

target_include_directories(core
PUBLIC
$<BUILD_INTERFACE:${PROMETHEUS_SRC_DIR}/core/include>
Expand Down
9 changes: 9 additions & 0 deletions contrib/prometheus-cpp-cmake/pull/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,18 @@ if(ENABLE_COMPRESSION)
endif()

add_library(pull
${PROMETHEUS_SRC_DIR}/pull/src/basic_auth.cc
${PROMETHEUS_SRC_DIR}/pull/src/basic_auth.h
${PROMETHEUS_SRC_DIR}/pull/src/endpoint.cc
${PROMETHEUS_SRC_DIR}/pull/src/endpoint.h
${PROMETHEUS_SRC_DIR}/pull/src/exposer.cc
${PROMETHEUS_SRC_DIR}/pull/src/handler.cc
${PROMETHEUS_SRC_DIR}/pull/src/handler.h
${PROMETHEUS_SRC_DIR}/pull/src/metrics_collector.cc
${PROMETHEUS_SRC_DIR}/pull/src/metrics_collector.h

${PROMETHEUS_SRC_DIR}/pull/src/detail/base64.h

$<$<BOOL:${USE_THIRDPARTY_LIBRARIES}>:$<TARGET_OBJECTS:civetweb>>
)

Expand Down
2 changes: 2 additions & 0 deletions contrib/prometheus-cpp-cmake/push/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ if(NOT CURL_FOUND)
endif()

add_library(push
${PROMETHEUS_SRC_DIR}/push/src/curl_wrapper.cc
${PROMETHEUS_SRC_DIR}/push/src/curl_wrapper.h
${PROMETHEUS_SRC_DIR}/push/src/gateway.cc
)

Expand Down