diff --git a/bazel/BUILD b/bazel/BUILD index 0cdca5a06d7ff..5bdd7b7520cc2 100644 --- a/bazel/BUILD +++ b/bazel/BUILD @@ -155,6 +155,37 @@ config_setting( values = {"define": "tcmalloc=debug"}, ) +config_setting( + name = "gperftools_tcmalloc", + values = {"define": "tcmalloc=gperftools"}, +) + +# As select() can't be nested we need these specialized settings to avoid ambiguity when choosing +# tcmalloc's flavor for x86_64 builds. +config_setting( + name = "disable_tcmalloc_on_linux_x86_64", + values = { + "define": "tcmalloc=disabled", + "cpu": "k8", + }, +) + +config_setting( + name = "gperftools_tcmalloc_on_linux_x86_64", + values = { + "define": "tcmalloc=gperftools", + "cpu": "k8", + }, +) + +config_setting( + name = "debug_tcmalloc_on_linux_x86_64", + values = { + "define": "tcmalloc=debug", + "cpu": "k8", + }, +) + config_setting( name = "disable_signal_trace", values = {"define": "signal_trace=disabled"}, diff --git a/bazel/README.md b/bazel/README.md index aae81ce988cc6..36e8f7ebc3247 100644 --- a/bazel/README.md +++ b/bazel/README.md @@ -607,7 +607,8 @@ The following optional features can be disabled on the Bazel build command-line: * Google C++ gRPC client with `--define google_grpc=disabled` * Backtracing on signals with `--define signal_trace=disabled` * Active stream state dump on signals with `--define signal_trace=disabled` or `--define disable_object_dump_on_signal_trace=disabled` -* tcmalloc with `--define tcmalloc=disabled` +* tcmalloc with `--define tcmalloc=disabled`. Also you can choose Gperftools' implementation of + tcmalloc with `--define tcmalloc=gperftools` which is the default for non-x86 builds. * deprecated features with `--define deprecated_features=disabled` @@ -626,7 +627,8 @@ The following optional features can be enabled on the Bazel build command-line: `--define log_debug_assert_in_release=enabled`. The default behavior is to compile debug assertions out of release builds so that the condition is not evaluated. This option has no effect in debug builds. * memory-debugging (scribbling over memory after allocation and before freeing) with - `--define tcmalloc=debug`. Note this option cannot be used with FIPS-compliant mode BoringSSL. + `--define tcmalloc=debug`. Note this option cannot be used with FIPS-compliant mode BoringSSL and + tcmalloc is built from the sources of Gperftools. * Default [path normalization](https://github.com/envoyproxy/envoy/issues/6435) with `--define path_normalization_by_default=true`. Note this still could be disable by explicit xDS config. * Manual stamping via VersionInfo with `--define manual_stamp=manual_stamp`. diff --git a/bazel/envoy_internal.bzl b/bazel/envoy_internal.bzl index f15af170819cb..5ad86609a00d4 100644 --- a/bazel/envoy_internal.bzl +++ b/bazel/envoy_internal.bzl @@ -60,10 +60,13 @@ def envoy_copts(repository, test = False): "//conditions:default": [], }) + select({ repository + "//bazel:disable_tcmalloc": ["-DABSL_MALLOC_HOOK_MMAP_DISABLE"], - "//conditions:default": ["-DTCMALLOC"], - }) + select({ - repository + "//bazel:debug_tcmalloc": ["-DENVOY_MEMORY_DEBUG_ENABLED=1"], - "//conditions:default": [], + repository + "//bazel:disable_tcmalloc_on_linux_x86_64": ["-DABSL_MALLOC_HOOK_MMAP_DISABLE"], + repository + "//bazel:gperftools_tcmalloc": ["-DGPERFTOOLS_TCMALLOC"], + repository + "//bazel:gperftools_tcmalloc_on_linux_x86_64": ["-DGPERFTOOLS_TCMALLOC"], + repository + "//bazel:debug_tcmalloc": ["-DENVOY_MEMORY_DEBUG_ENABLED=1", "-DGPERFTOOLS_TCMALLOC"], + repository + "//bazel:debug_tcmalloc_on_linux_x86_64": ["-DENVOY_MEMORY_DEBUG_ENABLED=1", "-DGPERFTOOLS_TCMALLOC"], + repository + "//bazel:linux_x86_64": ["-DTCMALLOC"], + "//conditions:default": ["-DGPERFTOOLS_TCMALLOC"], }) + select({ repository + "//bazel:disable_signal_trace": [], "//conditions:default": ["-DENVOY_HANDLE_SIGNALS"], @@ -118,6 +121,12 @@ def envoy_stdlib_deps(): def tcmalloc_external_dep(repository): return select({ repository + "//bazel:disable_tcmalloc": None, + repository + "//bazel:disable_tcmalloc_on_linux_x86_64": None, + repository + "//bazel:debug_tcmalloc": envoy_external_dep_path("gperftools"), + repository + "//bazel:debug_tcmalloc_on_linux_x86_64": envoy_external_dep_path("gperftools"), + repository + "//bazel:gperftools_tcmalloc": envoy_external_dep_path("gperftools"), + repository + "//bazel:gperftools_tcmalloc_on_linux_x86_64": envoy_external_dep_path("gperftools"), + repository + "//bazel:linux_x86_64": envoy_external_dep_path("tcmalloc"), "//conditions:default": envoy_external_dep_path("gperftools"), }) diff --git a/bazel/envoy_library.bzl b/bazel/envoy_library.bzl index bf0c89773fcb5..5eb90df500c04 100644 --- a/bazel/envoy_library.bzl +++ b/bazel/envoy_library.bzl @@ -20,6 +20,12 @@ load( def tcmalloc_external_deps(repository): return select({ repository + "//bazel:disable_tcmalloc": [], + repository + "//bazel:disable_tcmalloc_on_linux_x86_64": [], + repository + "//bazel:debug_tcmalloc": [envoy_external_dep_path("gperftools")], + repository + "//bazel:debug_tcmalloc_on_linux_x86_64": [envoy_external_dep_path("gperftools")], + repository + "//bazel:gperftools_tcmalloc": [envoy_external_dep_path("gperftools")], + repository + "//bazel:gperftools_tcmalloc_on_linux_x86_64": [envoy_external_dep_path("gperftools")], + repository + "//bazel:linux_x86_64": [envoy_external_dep_path("tcmalloc")], "//conditions:default": [envoy_external_dep_path("gperftools")], }) diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 3598973067931..64d61ea499404 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -197,6 +197,7 @@ def envoy_dependencies(skip_targets = []): _com_github_google_benchmark() _com_github_google_jwt_verify() _com_github_google_libprotobuf_mutator() + _com_github_google_tcmalloc() _com_github_gperftools_gperftools() _com_github_grpc_grpc() _com_github_jbeder_yaml_cpp() @@ -875,6 +876,16 @@ def _com_github_moonjit_moonjit(): actual = "@envoy//bazel/foreign_cc:moonjit", ) +def _com_github_google_tcmalloc(): + _repository_impl( + name = "com_github_google_tcmalloc", + ) + + native.bind( + name = "tcmalloc", + actual = "@com_github_google_tcmalloc//tcmalloc", + ) + def _com_github_gperftools_gperftools(): location = _get_location("com_github_gperftools_gperftools") http_archive( diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index f5dc9ccc5527f..6eba5a821d1d4 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -241,6 +241,18 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( last_updated = "2020-08-18", use_category = ["test_only"], ), + com_github_google_tcmalloc = dict( + project_name = "tcmalloc", + project_desc = "Fast, multi-threaded malloc implementation", + project_url = "https://github.com/google/tcmalloc", + version = "d1311bf409db47c3441d3de6ea07d768c6551dec", + sha256 = "e22444b6544edd81f11c987dd5e482a2e00bbff717badb388779ca57525dad50", + strip_prefix = "tcmalloc-{version}", + urls = ["https://github.com/google/tcmalloc/archive/{version}.tar.gz"], + use_category = ["dataplane_core", "controlplane"], + last_updated = "2020-09-16", + cpe = "N/A", + ), com_github_gperftools_gperftools = dict( project_name = "gperftools", project_desc = "tcmalloc and profiling libraries", diff --git a/ci/do_ci.sh b/ci/do_ci.sh index 4d8324d082840..f470f60c19e97 100755 --- a/ci/do_ci.sh +++ b/ci/do_ci.sh @@ -282,6 +282,7 @@ elif [[ "$CI_TARGET" == "bazel.compile_time_options" ]]; then "--define" "path_normalization_by_default=true" "--define" "deprecated_features=disabled" "--define" "use_new_codecs_in_integration_tests=true" + "--define" "tcmalloc=gperftools" "--define" "zlib=ng") ENVOY_STDLIB="${ENVOY_STDLIB:-libstdc++}" @@ -330,7 +331,8 @@ elif [[ "$CI_TARGET" == "bazel.coverage" || "$CI_TARGET" == "bazel.fuzz_coverage [[ "$CI_TARGET" == "bazel.fuzz_coverage" ]] && export FUZZ_COVERAGE=true - BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS[*]}" test/run_envoy_bazel_coverage.sh "${COVERAGE_TEST_TARGETS[@]}" + # We use custom BAZEL_BUILD_OPTIONS here to cover profiler's code. + BAZEL_BUILD_OPTIONS="${BAZEL_BUILD_OPTIONS[*]} --define tcmalloc=gperftools" test/run_envoy_bazel_coverage.sh "${COVERAGE_TEST_TARGETS[@]}" collect_build_profile coverage exit 0 elif [[ "$CI_TARGET" == "bazel.clang_tidy" ]]; then diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index a3b4de11bbf82..c8e6aa21469dc 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -47,6 +47,7 @@ Minor Behavior Changes * logging: nghttp2 log messages no longer appear at trace level unless `ENVOY_NGHTTP2_TRACE` is set in the environment. * lua: changed the response body returned by `httpCall()` API to raw data. Previously, the returned data was string. +* memory: switched to the `new tcmalloc `_ for linux_x86_64 builds. The `old tcmalloc `_ can still be enabled with the `--define tcmalloc=gperftools` option. * postgres: changed log format to tokenize fields of Postgres messages. * router: added transport failure reason to response body when upstream reset happens. After this change, the response body will be of the form `upstream connect error or disconnect/reset before headers. reset reason:{}, transport failure reason:{}`.This behavior may be reverted by setting runtime feature `envoy.reloadable_features.http_transport_failure_reason_in_body` to false. * router: now consumes all retry related headers to prevent them from being propagated to the upstream. This behavior may be reverted by setting runtime feature `envoy.reloadable_features.consume_all_retry_headers` to false. diff --git a/source/common/memory/stats.cc b/source/common/memory/stats.cc index 657f9b6b9b080..5b4e97261a4c9 100644 --- a/source/common/memory/stats.cc +++ b/source/common/memory/stats.cc @@ -4,7 +4,52 @@ #include "common/common/logger.h" -#ifdef TCMALLOC +#if defined(TCMALLOC) + +#include "tcmalloc/malloc_extension.h" + +namespace Envoy { +namespace Memory { + +uint64_t Stats::totalCurrentlyAllocated() { + return tcmalloc::MallocExtension::GetNumericProperty("generic.current_allocated_bytes") + .value_or(0); +} + +uint64_t Stats::totalCurrentlyReserved() { + // In Google's tcmalloc the semantics of generic.heap_size has + // changed: it doesn't include unmapped bytes. + return tcmalloc::MallocExtension::GetNumericProperty("generic.heap_size").value_or(0) + + tcmalloc::MallocExtension::GetNumericProperty("tcmalloc.pageheap_unmapped_bytes") + .value_or(0); +} + +uint64_t Stats::totalThreadCacheBytes() { + return tcmalloc::MallocExtension::GetNumericProperty("tcmalloc.current_total_thread_cache_bytes") + .value_or(0); +} + +uint64_t Stats::totalPageHeapFree() { + return tcmalloc::MallocExtension::GetNumericProperty("tcmalloc.pageheap_free_bytes").value_or(0); +} + +uint64_t Stats::totalPageHeapUnmapped() { + return tcmalloc::MallocExtension::GetNumericProperty("tcmalloc.pageheap_unmapped_bytes") + .value_or(0); +} + +uint64_t Stats::totalPhysicalBytes() { + return tcmalloc::MallocExtension::GetProperties()["generic.physical_memory_used"].value; +} + +void Stats::dumpStatsToLog() { + ENVOY_LOG_MISC(debug, "TCMalloc stats:\n{}", tcmalloc::MallocExtension::GetStats()); +} + +} // namespace Memory +} // namespace Envoy + +#elif defined(GPERFTOOLS_TCMALLOC) #include "gperftools/malloc_extension.h" @@ -74,4 +119,4 @@ void Stats::dumpStatsToLog() {} } // namespace Memory } // namespace Envoy -#endif // #ifdef TCMALLOC +#endif // #if defined(TCMALLOC) diff --git a/source/common/memory/utils.cc b/source/common/memory/utils.cc index 2fa957572217a..c6ac4f6c5fe87 100644 --- a/source/common/memory/utils.cc +++ b/source/common/memory/utils.cc @@ -3,15 +3,26 @@ #include "common/common/assert.h" #include "common/memory/stats.h" -#ifdef TCMALLOC +#if defined(TCMALLOC) +#include "tcmalloc/malloc_extension.h" +#elif defined(GPERFTOOLS_TCMALLOC) #include "gperftools/malloc_extension.h" #endif namespace Envoy { namespace Memory { +namespace { +#if defined(TCMALLOC) || defined(GPERFTOOLS_TCMALLOC) +// TODO(zyfjeff): Make max unfreed memory byte configurable +constexpr uint64_t MAX_UNFREED_MEMORY_BYTE = 100 * 1024 * 1024; +#endif +} // namespace + void Utils::releaseFreeMemory() { -#ifdef TCMALLOC +#if defined(TCMALLOC) + tcmalloc::MallocExtension::ReleaseMemoryToSystem(MAX_UNFREED_MEMORY_BYTE); +#elif defined(GPERFTOOLS_TCMALLOC) MallocExtension::instance()->ReleaseFreeMemory(); #endif } @@ -23,9 +34,7 @@ void Utils::releaseFreeMemory() { Ref: https://github.com/envoyproxy/envoy/pull/9471#discussion_r363825985 */ void Utils::tryShrinkHeap() { -#ifdef TCMALLOC - // TODO(zyfjeff): Make max unfreed memory byte configurable - static const uint64_t MAX_UNFREED_MEMORY_BYTE = 100 * 1024 * 1024; +#if defined(TCMALLOC) || defined(GPERFTOOLS_TCMALLOC) auto total_physical_bytes = Stats::totalPhysicalBytes(); auto allocated_size_by_app = Stats::totalCurrentlyAllocated(); diff --git a/source/common/profiler/profiler.h b/source/common/profiler/profiler.h index fdf4b20ee8f96..057ffda6f271d 100644 --- a/source/common/profiler/profiler.h +++ b/source/common/profiler/profiler.h @@ -2,10 +2,10 @@ #include -// Profiling support is provided in the release tcmalloc, but not in the library +// Profiling support is provided in the release tcmalloc of `gperftools`, but not in the library // that supplies the debug tcmalloc. So all the profiling code must be ifdef'd // on PROFILER_AVAILABLE which is dependent on those two settings. -#if defined(TCMALLOC) && !defined(ENVOY_MEMORY_DEBUG_ENABLED) +#if defined(GPERFTOOLS_TCMALLOC) && !defined(ENVOY_MEMORY_DEBUG_ENABLED) #define PROFILER_AVAILABLE #endif diff --git a/source/extensions/quic_listeners/quiche/platform/quic_mem_slice_impl.cc b/source/extensions/quic_listeners/quiche/platform/quic_mem_slice_impl.cc index c75f99c0bafbe..903ee1332d04e 100644 --- a/source/extensions/quic_listeners/quiche/platform/quic_mem_slice_impl.cc +++ b/source/extensions/quic_listeners/quiche/platform/quic_mem_slice_impl.cc @@ -16,7 +16,7 @@ QuicMemSliceImpl::QuicMemSliceImpl(QuicUniqueBufferPtr buffer, size_t length) : fragment_(std::make_unique( buffer.release(), length, [](const void* p, size_t, const Envoy::Buffer::BufferFragmentImpl*) { - delete static_cast(p); + delete[] static_cast(p); })) { single_slice_buffer_.addBufferFragment(*fragment_); ASSERT(this->length() == length); diff --git a/test/common/memory/heap_shrinker_test.cc b/test/common/memory/heap_shrinker_test.cc index 9b921847f671a..346a74899ff97 100644 --- a/test/common/memory/heap_shrinker_test.cc +++ b/test/common/memory/heap_shrinker_test.cc @@ -67,7 +67,7 @@ TEST_F(HeapShrinkerTest, ShrinkWhenTriggered) { const uint64_t physical_mem_after_shrink = Stats::totalCurrentlyReserved() - Stats::totalPageHeapUnmapped(); -#ifdef TCMALLOC +#if defined(TCMALLOC) || defined(GPERFTOOLS_TCMALLOC) EXPECT_GE(physical_mem_before_shrink, physical_mem_after_shrink); #else EXPECT_EQ(physical_mem_before_shrink, physical_mem_after_shrink); diff --git a/test/common/stats/stat_test_utility.cc b/test/common/stats/stat_test_utility.cc index 7cdbc08ab4dc5..a47c84d346f17 100644 --- a/test/common/stats/stat_test_utility.cc +++ b/test/common/stats/stat_test_utility.cc @@ -101,7 +101,7 @@ void forEachSampleStat(int num_clusters, std::function } MemoryTest::Mode MemoryTest::mode() { -#if !defined(TCMALLOC) || defined(ENVOY_MEMORY_DEBUG_ENABLED) +#if !(defined(TCMALLOC) || defined(GPERFTOOLS_TCMALLOC)) || defined(ENVOY_MEMORY_DEBUG_ENABLED) // We can only test absolute memory usage if the malloc library is a known // quantity. This decision is centralized here. As the preferred malloc // library for Envoy is TCMALLOC that's what we test for here. If we switch diff --git a/test/common/stats/symbol_table_impl_test.cc b/test/common/stats/symbol_table_impl_test.cc index d8d8db14d999b..5ac09db06a4ba 100644 --- a/test/common/stats/symbol_table_impl_test.cc +++ b/test/common/stats/symbol_table_impl_test.cc @@ -732,11 +732,9 @@ TEST(SymbolTableTest, Memory) { } } - // Make sure we don't regress. Data as of 2019/05/29: - // - // string_mem_used: 6710912 (libc++), 7759488 (libstdc++). + // Make sure we don't regress. + // Data as of 2019/05/29: // symbol_table_mem_used: 1726056 (3.9x) -- does not seem to depend on STL sizes. - EXPECT_MEMORY_LE(string_mem_used, 7759488); EXPECT_MEMORY_LE(symbol_table_mem_used, string_mem_used / 3); EXPECT_MEMORY_EQ(symbol_table_mem_used, 1726056); } diff --git a/test/common/stats/thread_local_store_test.cc b/test/common/stats/thread_local_store_test.cc index 18004e4ce5ec5..395a84cf32e6f 100644 --- a/test/common/stats/thread_local_store_test.cc +++ b/test/common/stats/thread_local_store_test.cc @@ -1180,7 +1180,7 @@ TEST_F(StatsThreadLocalStoreTestNoFixture, MemoryWithTlsRealSymbolTable) { TestUtil::MemoryTest memory_test; TestUtil::forEachSampleStat( 100, [this](absl::string_view name) { store_.counterFromString(std::string(name)); }); - EXPECT_MEMORY_EQ(memory_test.consumedBytes(), 827632); // July 20, 2020 + EXPECT_MEMORY_EQ(memory_test.consumedBytes(), 827616); // Sep 25, 2020 EXPECT_MEMORY_LE(memory_test.consumedBytes(), 0.9 * million_); } diff --git a/test/exe/main_common_test.cc b/test/exe/main_common_test.cc index 39d0486683d7d..000142b7ff2ea 100644 --- a/test/exe/main_common_test.cc +++ b/test/exe/main_common_test.cc @@ -31,6 +31,23 @@ using testing::Return; namespace Envoy { +namespace { + +#if !(defined(__clang_analyzer__) || \ + (defined(__has_feature) && \ + (__has_feature(thread_sanitizer) || __has_feature(address_sanitizer) || \ + __has_feature(memory_sanitizer)))) +const std::string& outOfMemoryPattern() { +#if defined(TCMALLOC) + CONSTRUCT_ON_FIRST_USE(std::string, ".*Unable to allocate.*"); +#else + CONSTRUCT_ON_FIRST_USE(std::string, ".*panic: out of memory.*"); +#endif +} +#endif + +} // namespace + /** * Captures common functions needed for invoking MainCommon.Maintains * an argv array that is terminated with nullptr. Identifies the config @@ -177,7 +194,7 @@ TEST_P(MainCommonDeathTest, OutOfMemoryHandler) { ENVOY_LOG_MISC(debug, "p={}", reinterpret_cast(p)); } }(), - ".*panic: out of memory.*"); + outOfMemoryPattern()); #endif } diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index 08e8019111a48..c264096681f38 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -267,6 +267,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSize) { // 2020/08/10 12275 37061 38000 Re-organize tls histogram maps to improve continuity. // 2020/08/11 12202 37061 38500 router: add new retry back-off strategy // 2020/09/11 12973 38993 upstream: predictive prefetch + // 2020/10/02 13251 39326 switch to google tcmalloc // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -287,7 +288,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSize) { // https://github.com/envoyproxy/envoy/issues/12209 // EXPECT_MEMORY_EQ(m_per_cluster, 37061); } - EXPECT_MEMORY_LE(m_per_cluster, 39000); // Round up to allow platform variations. + EXPECT_MEMORY_LE(m_per_cluster, 39350); // Round up to allow platform variations. } TEST_P(ClusterMemoryTestRunner, MemoryLargeHostSizeWithStats) {