memory: switch to the latest tcmalloc for x86_64 builds

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"},

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`.

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"),
     })
 

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")],
     })
 

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(

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",

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

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 <https://github.com/google/tcmalloc>`_ for linux_x86_64 builds. The `old tcmalloc <https://github.com/gperftools/gperftools>`_ 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.

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)

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();
 

source/common/profiler/profiler.h

@@ -2,10 +2,10 @@
 
 #include <string>
 
-// 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
 

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<Envoy::Buffer::BufferFragmentImpl>(
           buffer.release(), length,
           [](const void* p, size_t, const Envoy::Buffer::BufferFragmentImpl*) {
-            delete static_cast<const char*>(p);
+            delete[] static_cast<const char*>(p);
           })) {
   single_slice_buffer_.addBufferFragment(*fragment_);
   ASSERT(this->length() == length);

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);

test/common/stats/stat_test_utility.cc

@@ -101,7 +101,7 @@ void forEachSampleStat(int num_clusters, std::function<void(absl::string_view)>
 }
 
 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

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);
 }

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_);
 }
 

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<intptr_t>(p));
         }
       }(),
-      ".*panic: out of memory.*");
+      outOfMemoryPattern());
 #endif
 }
 

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) {