You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "JosiahWI (via GitHub)" <gi...@apache.org> on 2023/06/14 11:47:53 UTC
[GitHub] [trafficserver] JosiahWI opened a new pull request, #9842: Search for and link profiler in CMake build
JosiahWI opened a new pull request, #9842:
URL: https://github.com/apache/trafficserver/pull/9842
This looks for the gperftools profiler and links it if found. It also puts both profiler and TCMalloc targets under the gperftools namespace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] cmcfarlen commented on a diff in pull request #9842: Search for and link profiler in CMake build
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #9842:
URL: https://github.com/apache/trafficserver/pull/9842#discussion_r1229934507
##########
CMakeLists.txt:
##########
@@ -108,8 +109,8 @@ find_package(PCRE)
include(FindOpenSSL)
find_package(OpenSSL)
-find_package(profiler)
-if(profiler_FOUND)
+if(ENABLE_PROFILER)
+ find_package(profiler REQUIRED)
set(TS_HAS_PROFILER TRUE)
Review Comment:
`set(TS_HAS_PROFILER ${profiler_FOUND})`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] cmcfarlen commented on a diff in pull request #9842: Search for and link profiler in CMake build
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #9842:
URL: https://github.com/apache/trafficserver/pull/9842#discussion_r1229824333
##########
CMakeLists.txt:
##########
@@ -88,25 +88,36 @@ CHECK_INCLUDE_FILE(arpa/nameser_compat.h HAVE_ARPA_NAMESER_COMPAT_H)
CHECK_INCLUDE_FILE(siginfo.h HAVE_SIGINFO_H)
# Find libraries
-find_package(TSMallocReplacement QUIET MODULE)
-if(TSMallocReplacement_FOUND)
- link_libraries(ts::TSMallocReplacement)
-endif()
find_package(brotli)
if(brotli_FOUND)
set(HAVE_BROTLI_ENCODE_H TRUE)
endif()
+
find_package(hwloc)
if(hwloc_FOUND)
set(TS_USE_HWLOC TRUE)
endif()
+
find_package(LibLZMA)
if(LibLZMA_FOUND)
set(HAVE_LZMA_H TRUE)
endif()
+
find_package(PCRE)
+
include(FindOpenSSL)
find_package(OpenSSL)
+
+find_package(profiler)
+if(profiler_FOUND)
+ set(TS_HAS_PROFILER TRUE)
+endif()
Review Comment:
We should add an option for looking for the profiler as we don't always want to profile. The auto tools build has `--with-profiler` for this.
##########
CMakeLists.txt:
##########
@@ -88,25 +88,36 @@ CHECK_INCLUDE_FILE(arpa/nameser_compat.h HAVE_ARPA_NAMESER_COMPAT_H)
CHECK_INCLUDE_FILE(siginfo.h HAVE_SIGINFO_H)
# Find libraries
-find_package(TSMallocReplacement QUIET MODULE)
-if(TSMallocReplacement_FOUND)
- link_libraries(ts::TSMallocReplacement)
-endif()
find_package(brotli)
if(brotli_FOUND)
set(HAVE_BROTLI_ENCODE_H TRUE)
endif()
+
find_package(hwloc)
if(hwloc_FOUND)
set(TS_USE_HWLOC TRUE)
endif()
+
find_package(LibLZMA)
if(LibLZMA_FOUND)
set(HAVE_LZMA_H TRUE)
endif()
+
find_package(PCRE)
+
include(FindOpenSSL)
find_package(OpenSSL)
+
+find_package(profiler)
+if(profiler_FOUND)
+ set(TS_HAS_PROFILER TRUE)
+endif()
Review Comment:
Maybe just `PROFILE` as an option and default to off.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt merged pull request #9842: Search for and link profiler in CMake build
Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt merged PR #9842:
URL: https://github.com/apache/trafficserver/pull/9842
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] JosiahWI commented on a diff in pull request #9842: Search for and link profiler in CMake build
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on code in PR #9842:
URL: https://github.com/apache/trafficserver/pull/9842#discussion_r1229836949
##########
CMakeLists.txt:
##########
@@ -88,25 +88,36 @@ CHECK_INCLUDE_FILE(arpa/nameser_compat.h HAVE_ARPA_NAMESER_COMPAT_H)
CHECK_INCLUDE_FILE(siginfo.h HAVE_SIGINFO_H)
# Find libraries
-find_package(TSMallocReplacement QUIET MODULE)
-if(TSMallocReplacement_FOUND)
- link_libraries(ts::TSMallocReplacement)
-endif()
find_package(brotli)
if(brotli_FOUND)
set(HAVE_BROTLI_ENCODE_H TRUE)
endif()
+
find_package(hwloc)
if(hwloc_FOUND)
set(TS_USE_HWLOC TRUE)
endif()
+
find_package(LibLZMA)
if(LibLZMA_FOUND)
set(HAVE_LZMA_H TRUE)
endif()
+
find_package(PCRE)
+
include(FindOpenSSL)
find_package(OpenSSL)
+
+find_package(profiler)
+if(profiler_FOUND)
+ set(TS_HAS_PROFILER TRUE)
+endif()
Review Comment:
Maybe `ENABLE_PROFILER` would be better? The autotools build uses `--enable-xxx` for enabling/disabling features, and I think this shows the intent well. Note that the autotools build has no way to explicitly disable the profiler that I could find.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org