rolling back a problematic change

tcmalloc/static_forwarder.h

@@ -121,9 +121,7 @@ class StaticForwarder : private Parameters {
   [[nodiscard]] static void* absl_nonnull AllocCpuCache(
       size_t size, std::align_val_t alignment)
       ABSL_LOCKS_EXCLUDED(pageheap_lock) {
-    // No IsInited() check: Static::SlowInitIfNecessary calls
-    // CpuCache::Activate after initializing the arena but before setting
-    // IsInited().
+    TC_ASSERT(state.IsInited());
     // TODO(b/373944374): Arena is thread-safe, but we take the pageheap_lock to
     // present a consistent view of memory usage.
     PageHeapSpinLockHolder l;

tcmalloc/static_vars.cc

@@ -88,25 +88,6 @@ SizeClassConfiguration Static::size_class_configuration() {
   return SizeClassConfiguration::kReuseRelaxedBelow64;
 }
 
-namespace {
-
-bool DeferPerCpuCaches() {
-  const char* e = thread_safe_getenv("TCMALLOC_DEFER_PER_CPU_CACHES");
-  if (e != nullptr) {
-    if (e[0] == '\0' || (e[0] == '0' && e[1] == '\0')) {
-      return false;
-    }
-    if (e[0] == '1' && e[1] == '\0') {
-      return true;
-    }
-    TC_BUG("bad env var '%s'", e);
-  }
-
-  return false;
-}
-
-}  // namespace
-
 void Static::ActivateCpuCacheIfNecessary() {
   InitIfNecessary();
   if (CpuCacheActive()) {
@@ -131,50 +112,40 @@ ABSL_ATTRIBUTE_COLD ABSL_ATTRIBUTE_NOINLINE void Static::SlowInitIfNecessary() {
     return;
   }
 
-  {
-    PageHeapSpinLockHolder l;
-
-    numa_topology_.Init();
-    TC_CHECK(sizemap_.Init(SizeMap::CurrentClasses().classes));
-    system_allocator_.Init(numa_topology_, kMinMmapAlloc);
-    sampledallocation_allocator_.Init(arena_);
-    span_allocator_.Init(arena_);
-    threadcache_allocator_.Init(arena_);
-    linked_sample_allocator_.Init(arena_);
-    sampled_allocation_recorder_.Init(sampledallocation_allocator_);
-    peak_heap_tracker_.Init(sampledallocation_allocator_);
-
-    // Verify we can determine the number of CPUs now, since we will need it
-    // later for per-CPU caches and initializing the cache topology.
-    if (ABSL_PREDICT_FALSE(!NumCPUsMaybe().has_value())) {
-      TCMalloc_Internal_SetPerCpuCachesEnabledNoBuildRequirement(false);
-    }
-    (void)subtle::percpu::IsFast();
-    PerCpuState::state().Init();
-    CacheTopology::Instance().Init();
-    cpu_cache_.Init();
-
-    Parameters::InitializeFromExperiments();
-
-    // Do a bit of sanitizing: make sure central_cache is aligned properly
-    TC_CHECK_EQ((sizeof(transfer_cache_) % ABSL_CACHELINE_SIZE), 0);
-    transfer_cache_.Init();
-    // The constructor of the sharded transfer cache leaves it in a disabled
-    // state.
-    sharded_transfer_cache_.Init();
-    new (page_allocator_.memory) PageAllocator;
-    guardedpage_allocator_.Init(/*max_allocated_pages=*/64,
-                                /*total_pages=*/128);
-  }
-
-  if (!DeferPerCpuCaches() && Parameters::per_cpu_caches() &&
-      subtle::percpu::IsFast()) {
-    cpu_cache_.Activate();
-    cpu_cache_active_.store(true, std::memory_order_release);
+  PageHeapSpinLockHolder l;
+
+  numa_topology_.Init();
+  TC_CHECK(sizemap_.Init(SizeMap::CurrentClasses().classes));
+  system_allocator_.Init(numa_topology_, kMinMmapAlloc);
+  sampledallocation_allocator_.Init(arena_);
+  span_allocator_.Init(arena_);
+  threadcache_allocator_.Init(arena_);
+  linked_sample_allocator_.Init(arena_);
+  sampled_allocation_recorder_.Init(sampledallocation_allocator_);
+  peak_heap_tracker_.Init(sampledallocation_allocator_);
+
+  // Verify we can determine the number of CPUs now, since we will need it
+  // later for per-CPU caches and initializing the cache topology.
+  if (ABSL_PREDICT_FALSE(!NumCPUsMaybe().has_value())) {
+    TCMalloc_Internal_SetPerCpuCachesEnabledNoBuildRequirement(false);
   }
+  (void)subtle::percpu::IsFast();
+  PerCpuState::state().Init();
+  CacheTopology::Instance().Init();
+  cpu_cache_.Init();
+
+  Parameters::InitializeFromExperiments();
+
+  // Do a bit of sanitizing: make sure central_cache is aligned properly
+  TC_CHECK_EQ((sizeof(transfer_cache_) % ABSL_CACHELINE_SIZE), 0);
+  transfer_cache_.Init();
+  // The constructor of the sharded transfer cache leaves it in a disabled
+  // state.
+  sharded_transfer_cache_.Init();
+  new (page_allocator_.memory) PageAllocator;
+  guardedpage_allocator_.Init(/*max_allocated_pages=*/64,
+                              /*total_pages=*/128);
 
-  // Publish only after activation, so that threads racing to make their first
-  // allocation wait on init_lock_ rather than starting on per-thread caches.
   inited_.store(true, std::memory_order_release);
 }
 

tcmalloc/tcmalloc_variants.cmake

@@ -300,15 +300,6 @@ function(tcmalloc_cc_test_variants)
     DEPS ${TCMALLOC_DEPS} tcmalloc::tcmalloc_legacy_locking2 tcmalloc::common_legacy_locking2
     ENV ${TCMALLOC_ENV}
   )
-  tcmalloc_cc_test(NAME ${TCMALLOC_NAME}_defer_per_cpu_caches
-    ${EXTRA_ARGS}
-    SRCS ${TCMALLOC_SRCS}
-    HDRS ${TCMALLOC_HDRS}
-    COPTS ${TCMALLOC_COPTS}
-    LINKOPTS ${TCMALLOC_LINKOPTS}
-    DEPS ${TCMALLOC_DEPS} tcmalloc::tcmalloc tcmalloc::common_8k_pages
-    ENV ${TCMALLOC_ENV} "TCMALLOC_DEFER_PER_CPU_CACHES=1"
-  )
   tcmalloc_cc_test(NAME ${TCMALLOC_NAME}_tcmalloc_cfl_prioritization_ablation
     ${EXTRA_ARGS}
     SRCS ${TCMALLOC_SRCS}
@@ -559,13 +550,6 @@ function(tcmalloc_cc_binary_variants)
     LINKOPTS ${TCMALLOC_LINKOPTS}
     DEPS ${TCMALLOC_DEPS} tcmalloc::tcmalloc_legacy_locking2 tcmalloc::common_legacy_locking2
   )
-  tcmalloc_cc_binary(NAME ${TCMALLOC_NAME}_defer_per_cpu_caches
-    SRCS ${TCMALLOC_SRCS}
-    HDRS ${TCMALLOC_HDRS}
-    COPTS ${TCMALLOC_COPTS}
-    LINKOPTS ${TCMALLOC_LINKOPTS}
-    DEPS ${TCMALLOC_DEPS} tcmalloc::tcmalloc tcmalloc::common_8k_pages
-  )
   tcmalloc_cc_binary(NAME ${TCMALLOC_NAME}_tcmalloc_cfl_prioritization_ablation
     SRCS ${TCMALLOC_SRCS}
     HDRS ${TCMALLOC_HDRS}

tcmalloc/testing/BUILD

@@ -513,7 +513,6 @@ create_tcmalloc_testsuite(
     deps = [
         ":rseq_util",
         "//tcmalloc:malloc_extension",
-        "//tcmalloc/internal:environment",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/strings:string_view",
@@ -526,11 +525,8 @@ create_tcmalloc_testsuite(
     copts = TCMALLOC_DEFAULT_COPTS,
     linkstatic = 1,
     deps = [
-        ":rseq_util",
         "//tcmalloc:malloc_extension",
         "//tcmalloc/internal:config",
-        "//tcmalloc/internal:environment",
-        "//tcmalloc/internal:parameter_accessors",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_googletest//:gtest_main",
     ],
@@ -542,11 +538,8 @@ create_tcmalloc_testsuite(
     copts = TCMALLOC_DEFAULT_COPTS + ["-DFIRST_ALLOCATION_LARGE"],
     linkstatic = 1,
     deps = [
-        ":rseq_util",
         "//tcmalloc:malloc_extension",
         "//tcmalloc/internal:config",
-        "//tcmalloc/internal:environment",
-        "//tcmalloc/internal:parameter_accessors",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_googletest//:gtest_main",
     ],
@@ -560,7 +553,6 @@ create_tcmalloc_testsuite(
     deps = [
         ":rseq_util",
         "//tcmalloc:malloc_extension",
-        "//tcmalloc/internal:environment",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/strings:str_format",
         "@com_google_absl//absl/strings:string_view",

tcmalloc/testing/CMakeLists.txt

@@ -472,7 +472,6 @@ tcmalloc_cc_test_variants(
     "absl::core_headers"
     "absl::str_format"
     "absl::string_view"
-    "tcmalloc::internal_environment"
     "tcmalloc::malloc_extension"
     "tcmalloc::testing_rseq_util"
 )
@@ -488,10 +487,7 @@ tcmalloc_cc_test_variants(
     "GTest::gmock"
     "absl::core_headers"
     "tcmalloc::internal_config"
-    "tcmalloc::internal_environment"
-    "tcmalloc::internal_parameter_accessors"
     "tcmalloc::malloc_extension"
-    "tcmalloc::testing_rseq_util"
 )
 
 tcmalloc_cc_test_variants(
@@ -507,10 +503,7 @@ tcmalloc_cc_test_variants(
     "GTest::gmock"
     "absl::core_headers"
     "tcmalloc::internal_config"
-    "tcmalloc::internal_environment"
-    "tcmalloc::internal_parameter_accessors"
     "tcmalloc::malloc_extension"
-    "tcmalloc::testing_rseq_util"
 )
 
 tcmalloc_cc_test_variants(
@@ -522,7 +515,6 @@ tcmalloc_cc_test_variants(
     "absl::core_headers"
     "absl::str_format"
     "absl::string_view"
-    "tcmalloc::internal_environment"
     "tcmalloc::malloc_extension"
     "tcmalloc::testing_rseq_util"
 )

tcmalloc/testing/first_allocation_test.cc

@@ -12,17 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 //
-// Exercises the first allocation in the process, which initializes TCMalloc,
-// while a second thread races to allocate as soon as initialization is
-// published.  The allocations are made from a priority-101 constructor: after
-// libc is initialized (unlike .preinit_array, which is too early for TCMalloc's
-// initialization), but before default-priority constructors such as TCMalloc's
-// own.
+// Exercises the first allocation in the process, which initializes TCMalloc.
+// It is made from a priority-101 constructor: after libc is initialized
+// (unlike .preinit_array, which is too early for TCMalloc's initialization),
+// but before default-priority constructors such as TCMalloc's own.
 
-#include <pthread.h>
 #include <stddef.h>
-#include <stdint.h>
-#include <stdlib.h>
 
 #include <new>
 #include <optional>
@@ -31,12 +26,7 @@
 #include "absl/base/attributes.h"
 #include "tcmalloc/common.h"
 #include "tcmalloc/internal/config.h"
-#include "tcmalloc/internal/environment.h"
-#include "tcmalloc/internal/parameter_accessors.h"
 #include "tcmalloc/malloc_extension.h"
-#include "tcmalloc/parameters.h"
-#include "tcmalloc/static_vars.h"
-#include "tcmalloc/testing/rseq_util.h"
 
 namespace tcmalloc::tcmalloc_internal {
 namespace {
@@ -50,55 +40,11 @@ constexpr size_t kSize = 1000;
 #endif
 
 ABSL_CONST_INIT std::optional<size_t> first_allocation_size;
-ABSL_CONST_INIT bool active_after_first_allocation = false;
-ABSL_CONST_INIT bool active_when_second_thread_saw_inited = false;
 
-void* SecondThread(void*) {
-  // Allocate as soon as initialization is published.
-  while (!tc_globals.IsInited()) {
-  }
-  active_when_second_thread_saw_inited = tc_globals.CpuCacheActive();
-  sized_ptr_t res = __size_returning_new(kSize);
-  ::operator delete(res.p, res.n);
-  return nullptr;
-}
-
-__attribute__((constructor(101))) void FirstAllocations() {
-  if (kSanitizerPresent) {
-    // TCMalloc is not the allocator, so the second thread would never see it
-    // initialized.
-    return;
-  }
-
-  // Keep the first allocations unsampled: a guarded sample returns the
-  // requested size rather than the size class.
-  const int64_t sampling_interval = Parameters::profile_sampling_interval();
-  TCMalloc_Internal_SetProfileSamplingInterval(0);
-
-  pthread_t second_thread;
-  if (pthread_create(&second_thread, nullptr, SecondThread, nullptr) != 0) {
-    abort();
-  }
+__attribute__((constructor(101))) void FirstAllocation() {
   sized_ptr_t res = __size_returning_new(kSize);
-  active_after_first_allocation = tc_globals.CpuCacheActive();
   first_allocation_size = res.n;
   ::operator delete(res.p, res.n);
-  pthread_join(second_thread, nullptr);
-
-  TCMalloc_Internal_SetProfileSamplingInterval(sampling_interval);
-}
-
-// Per-CPU caches are expected on the first allocation unless the build is
-// per-thread, rseq is unavailable, or TCMALLOC_DEFER_PER_CPU_CACHES=1 defers
-// activation.
-bool ExpectActiveOnFirstAllocation() {
-#if defined(TCMALLOC_DEPRECATED_PERTHREAD)
-  return false;
-#else
-  const char* defer = thread_safe_getenv("TCMALLOC_DEFER_PER_CPU_CACHES");
-  const bool deferred = defer != nullptr && defer[0] == '1' && defer[1] == '\0';
-  return testing::ExpectWorkingRseq() && !deferred;
-#endif
 }
 
 TEST(FirstAllocationTest, UsesInitializedSizeClass) {
@@ -110,16 +56,5 @@ TEST(FirstAllocationTest, UsesInitializedSizeClass) {
             MallocExtension::GetEstimatedAllocatedSize(kSize));
 }
 
-TEST(FirstAllocationTest, ActivatesPerCpuCaches) {
-  if (kSanitizerPresent) {
-    GTEST_SKIP() << "TCMalloc is not the allocator";
-  }
-  const bool expected = ExpectActiveOnFirstAllocation();
-  EXPECT_EQ(active_after_first_allocation, expected);
-  // A thread that sees TCMalloc initialized must not start on a per-thread
-  // cache while the first allocation is still activating per-CPU caches.
-  EXPECT_EQ(active_when_second_thread_saw_inited, expected);
-}
-
 }  // namespace
 }  // namespace tcmalloc::tcmalloc_internal

tcmalloc/testing/no_deps_test.cc

@@ -32,7 +32,6 @@
 #include "absl/base/macros.h"
 #include "absl/strings/str_format.h"
 #include "absl/strings/string_view.h"
-#include "tcmalloc/internal/environment.h"
 #include "tcmalloc/internal_malloc_extension.h"
 #include "tcmalloc/malloc_extension.h"
 #include "tcmalloc/testing/rseq_util.h"
@@ -42,9 +41,6 @@ const size_t kMin = 8;
 void* blocks[kMem / kMin];
 
 int main() {
-  // TCMallocGuard's allocation before main() activates per-CPU caches unless
-  // deferred.
-
   if (&TCMalloc_Internal_ForceCpuCacheActivation != nullptr) {
     TCMalloc_Internal_ForceCpuCacheActivation();
   }

tcmalloc/variants.bzl

@@ -241,12 +241,6 @@ test_variants = [
         "copts": ["-DTCMALLOC_INTERNAL_8K_PAGES", "-DTCMALLOC_INTERNAL_LEGACY_LOCKING2"],
         "tags": ["noubsan"],
     },
-    {
-        "name": "defer_per_cpu_caches",
-        "malloc": "//tcmalloc",
-        "deps": ["//tcmalloc:common_8k_pages"],
-        "env": {"TCMALLOC_DEFER_PER_CPU_CACHES": "1"},
-    },
 ] + experiment_variants
 
 def create_tcmalloc_library(