Skip to content

Tcmalloc version updateupdate#38114

Closed
krinkinmu wants to merge 1 commit intoenvoyproxy:mainfrom
krinkinmu:tcmalloc-update
Closed

Tcmalloc version updateupdate#38114
krinkinmu wants to merge 1 commit intoenvoyproxy:mainfrom
krinkinmu:tcmalloc-update

Conversation

@krinkinmu
Copy link
Copy Markdown
Contributor

Commit Message:

Refresh the version of tcmalloc
I'm trying to build Envoy with Clang-18 and the version of tcmalloc
we currently uses trips thread safety analysis, which generates a
warning and ultimately results in a build failure.

I could disable the warning, but I think thread safety analysis warnings
might be potentially useful. Plus, the issue is addressed in a later
version of tcmalloc and the version we use is over 2 years old by now.

I figured the best way to address the warning then might be to refresh
the version of tcmalloc Envoy uses.

This change bumps it to the commit that contained the fix for the
warning I want to address, though I'm not avert to using even more fresh
version either if there is interest in that.

Additional Description: Related to the work in #37911
Risk Level: Low to Medium (I guess tcmalloc is used everywhere)
Testing: build + ./ci/do_ci.sh release.test_only
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a

+cc @phlax

I'm trying to build Envoy with Clang-18 and the version of tcmalloc
we currently uses trips thread safety analysis, which generates a
warning and ultimately results in a build failure.

I could disable the warning, but I think thread safety analysis warnings
might be potentially useful. Plus, the issue is addressed in a later
version of tcmalloc and the version we use is over 2 years old by now.

I figured the best way to address the warning then might be to refresh
the version of tcmalloc Envoy uses.

This change bumps it to the commit that contained the fix for the
warning I want to address, though I'm not avert to using even more fresh
version either if there is interest in that.

Signed-off-by: Mikhail Krinkin <mkrinkin@microsoft.com>
@repokitteh-read-only repokitteh-read-only Bot added the deps Approval required for changes to Envoy's external dependencies label Jan 20, 2025
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #38114 was opened by krinkinmu.

see: more, trace.

@moderation
Copy link
Copy Markdown
Contributor

It is a shame that Tcmalloc doesn't do releases or tags and is just a stream of commits. From our current version to the commit you picked, the diff is https://github.com/google/tcmalloc/compare/e33c7bc..4ef8fe1. The commit is almost a year after the current commit but still old at Oct 13 2023. It would be good to try to bump to something newer or if possible the latest however that diff is enormous - https://github.com/google/tcmalloc/compare/e33c7bc..e780ef2

Related:

@krinkinmu
Copy link
Copy Markdown
Contributor Author

gcc build failure seem to be a known issue with tcmalloc code using some new-ish feature of as that apparently is not supported by binutils version used in our Envoy CI image (see google/tcmalloc#222). Let me hold off this PR for now, until I can sort it out.

@moderation I'm happy to update to a newer version. It seems like this version has problems anyways, so I need to dig a bit more and check it with different compilers and compiler versions.

@krinkinmu krinkinmu closed this Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps Approval required for changes to Envoy's external dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants