diff --git a/tcmalloc/static_forwarder.h b/tcmalloc/static_forwarder.h index 1db2cde09..f390042e2 100644 --- a/tcmalloc/static_forwarder.h +++ b/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; diff --git a/tcmalloc/static_vars.cc b/tcmalloc/static_vars.cc index a0e6a0044..92ffda8f2 100644 --- a/tcmalloc/static_vars.cc +++ b/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); } diff --git a/tcmalloc/tcmalloc_variants.cmake b/tcmalloc/tcmalloc_variants.cmake index d2bb07e4d..8578493c9 100644 --- a/tcmalloc/tcmalloc_variants.cmake +++ b/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} diff --git a/tcmalloc/testing/BUILD b/tcmalloc/testing/BUILD index ab80b3b38..be9217304 100644 --- a/tcmalloc/testing/BUILD +++ b/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", diff --git a/tcmalloc/testing/CMakeLists.txt b/tcmalloc/testing/CMakeLists.txt index b638dd70d..2efff7ea7 100644 --- a/tcmalloc/testing/CMakeLists.txt +++ b/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" ) diff --git a/tcmalloc/testing/first_allocation_test.cc b/tcmalloc/testing/first_allocation_test.cc index aa3864caf..5acbcf3bd 100644 --- a/tcmalloc/testing/first_allocation_test.cc +++ b/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 #include -#include -#include #include #include @@ -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 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 diff --git a/tcmalloc/testing/no_deps_test.cc b/tcmalloc/testing/no_deps_test.cc index 6ac3a0874..e360956de 100644 --- a/tcmalloc/testing/no_deps_test.cc +++ b/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(); } diff --git a/tcmalloc/variants.bzl b/tcmalloc/variants.bzl index 1fad61ea4..5cdfeed97 100644 --- a/tcmalloc/variants.bzl +++ b/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(