cmake: locate tcmalloc via gperftools header and library#473
Open
GauthamPrabhuM wants to merge 1 commit into
Open
cmake: locate tcmalloc via gperftools header and library#473GauthamPrabhuM wants to merge 1 commit into
GauthamPrabhuM wants to merge 1 commit into
Conversation
The Findtcmalloc module searched for the header `google/tcmalloc.h`, a path that gperftools deprecated over a decade ago. Current releases ship `gperftools/tcmalloc.h`, so on modern systems `find_path` failed and, because TCMALLOC_INCLUDE_DIR was a required argument to FIND_PACKAGE_HANDLE_STANDARD_ARGS, tcmalloc was reported as not found even when libtcmalloc was present and installable. BioDynaMo only links against tcmalloc (-ltcmalloc) and never includes its headers, so the include directory is not needed to build. This: - searches for `gperftools/tcmalloc.h` first, keeping `google/tcmalloc.h` as a fallback (used solely as a hint to locate the pprof binary), and - bases detection on the library alone, dropping TCMALLOC_INCLUDE_DIR from the required arguments. Verified against gperftools 2.18.1 (which ships only the gperftools/ header): the previous module reported TCMALLOC_FOUND=FALSE despite libtcmalloc.dylib being found, while the updated module reports TCMALLOC_FOUND=TRUE. Fixes BioDynaMo#396 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #396.
cmake/Findtcmalloc.cmakelocated tcmalloc by searching for the headergoogle/tcmalloc.h— a path gperftools deprecated more than a decade ago.Current gperftools releases ship
gperftools/tcmalloc.hinstead, so on modernsystems
find_pathfailed. BecauseTCMALLOC_INCLUDE_DIRwas a requiredargument to
FIND_PACKAGE_HANDLE_STANDARD_ARGS, tcmalloc was reported as notfound even when
libtcmallocwas installed and linkable.As the issue reporter notes, BioDynaMo only links against tcmalloc
(
-ltcmalloc) and never#includes its headers, so the include directory isnot needed to build at all.
Changes
gperftools/tcmalloc.hfirst, keepinggoogle/tcmalloc.has afallback. The header is now used only as a hint to locate the
pprofbinary, not as a hard requirement.
TCMALLOC_INCLUDE_DIRfrom the required arguments ofFIND_PACKAGE_HANDLE_STANDARD_ARGS, so detection is based on the libraryalone.
I confirmed
TCMALLOC_INCLUDE_DIR/TCMALLOC_INCLUDE_DIRSare never consumedby any compile/include path in the project (only
TCMALLOC_FOUNDand the-ltcmalloclinker flags are used), so dropping the header requirement cannotaffect compilation.
Testing
Verified against gperftools 2.18.1, which ships only
gperftools/tcmalloc.h(no
google/tcmalloc.h), using a minimalfind_package(tcmalloc)harness:Could NOT find tcmalloc (missing: TCMALLOC_INCLUDE_DIR)→TCMALLOC_FOUND=FALSE(despitelibtcmalloc.dylibbeing found)Found tcmalloc: .../libtcmalloc.dylib;.../libprofiler.dylib→TCMALLOC_FOUND=TRUEThe change is confined to
cmake/Findtcmalloc.cmake;tcmallocisOFFbydefault, so default builds are unaffected.
🤖 Generated with Claude Code