Skip to content

Commit

Permalink
Roll back huge region demand-based release experiment.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 689025658
Change-Id: I0401a2def84751341655096ee484f19f6db290a7
  • Loading branch information
v-gogte authored and copybara-github committed Oct 23, 2024
1 parent 8bde231 commit eafcc39
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 40 deletions.
2 changes: 0 additions & 2 deletions tcmalloc/experiment_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ enum class Experiment : int {
TEST_ONLY_TCMALLOC_HUGE_CACHE_RELEASE_30S, // TODO(b/319872040): Complete experiment.
TEST_ONLY_TCMALLOC_REUSE_SIZE_CLASSES, // TODO(b/358126781): Complete experiment.
TCMALLOC_REUSE_SIZE_CLASSES, // TODO(b/358126781): Complete experiment.
TCMALLOC_HUGE_REGION_DEMAND_BASED_RELEASE, // TODO(b/328440160): Complete experiment.
TEST_ONLY_TCMALLOC_BIG_SPAN, // TODO(b/304135905): Complete experiment.
TCMALLOC_BIG_SPAN, // TODO(b/304135905): Complete experiment.
TEST_ONLY_L3_AWARE, // TODO(b/239977380): Complete experiment.
Expand All @@ -49,7 +48,6 @@ inline constexpr ExperimentConfig experiments[] = {
{Experiment::TEST_ONLY_TCMALLOC_HUGE_CACHE_RELEASE_30S, "TEST_ONLY_TCMALLOC_HUGE_CACHE_RELEASE_30S"},
{Experiment::TEST_ONLY_TCMALLOC_REUSE_SIZE_CLASSES, "TEST_ONLY_TCMALLOC_REUSE_SIZE_CLASSES"},
{Experiment::TCMALLOC_REUSE_SIZE_CLASSES, "TCMALLOC_REUSE_SIZE_CLASSES"},
{Experiment::TCMALLOC_HUGE_REGION_DEMAND_BASED_RELEASE, "TCMALLOC_HUGE_REGION_DEMAND_BASED_RELEASE"},
{Experiment::TEST_ONLY_TCMALLOC_BIG_SPAN, "TEST_ONLY_TCMALLOC_BIG_SPAN"},
{Experiment::TCMALLOC_BIG_SPAN, "TCMALLOC_BIG_SPAN"},
{Experiment::TEST_ONLY_L3_AWARE, "TEST_ONLY_L3_AWARE"},
Expand Down
25 changes: 4 additions & 21 deletions tcmalloc/parameters.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,21 +106,6 @@ static std::atomic<int64_t>& skip_subrelease_interval_ns() {
return v;
}

// As huge_region_demand_based_release_enabled() is determined at runtime, we
// cannot require constant initialization for the atomic. This avoids an
// initialization order fiasco.
static std::atomic<bool>& huge_region_demand_based_release_enabled() {
ABSL_CONST_INIT static absl::once_flag flag;
ABSL_CONST_INIT static std::atomic<bool> v{false};
absl::base_internal::LowLevelCallOnce(&flag, [&]() {
if (IsExperimentActive(
Experiment::TCMALLOC_HUGE_REGION_DEMAND_BASED_RELEASE)) {
v.store(true, std::memory_order_relaxed);
}
});
return v;
}

// Configures short and long intervals to zero by default. We expect to set them
// to the non-zero durations once the feature is no longer experimental.
static std::atomic<int64_t>& skip_subrelease_short_interval_ns() {
Expand Down Expand Up @@ -248,6 +233,9 @@ ABSL_CONST_INIT std::atomic<int64_t> Parameters::guarded_sampling_interval_(
// TODO(b/285379004): Remove this opt-out.
ABSL_CONST_INIT std::atomic<bool> Parameters::release_partial_alloc_pages_(
true);
// TODO(b/328440160): Remove this opt-out.
ABSL_CONST_INIT std::atomic<bool> Parameters::huge_region_demand_based_release_(
false);
// TODO(b/123345734): Remove the flag when experimentation is done.
ABSL_CONST_INIT std::atomic<bool> Parameters::resize_size_class_max_capacity_(
true);
Expand Down Expand Up @@ -320,11 +308,6 @@ absl::Duration Parameters::cache_demand_release_long_interval() {
cache_demand_release_long_interval_ns().load(std::memory_order_relaxed));
}

bool Parameters::huge_region_demand_based_release() {
return huge_region_demand_based_release_enabled().load(
std::memory_order_relaxed);
}

bool Parameters::dense_trackers_sorted_on_spans_allocated() {
ABSL_CONST_INIT static absl::once_flag flag;
ABSL_CONST_INIT static std::atomic<bool> v{false};
Expand Down Expand Up @@ -554,7 +537,7 @@ void TCMalloc_Internal_SetHugeCacheDemandBasedRelease(bool v) {
}

void TCMalloc_Internal_SetHugeRegionDemandBasedRelease(bool v) {
tcmalloc::tcmalloc_internal::huge_region_demand_based_release_enabled().store(
Parameters::huge_region_demand_based_release_.store(
v, std::memory_order_relaxed);
}

Expand Down
5 changes: 4 additions & 1 deletion tcmalloc/parameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ class Parameters {
return release_partial_alloc_pages_.load(std::memory_order_relaxed);
}

static bool huge_region_demand_based_release();
static bool huge_region_demand_based_release() {
return huge_region_demand_based_release_.load(std::memory_order_relaxed);
}

static bool huge_cache_demand_based_release() {
return huge_cache_demand_based_release_.load(std::memory_order_relaxed);
Expand Down Expand Up @@ -262,6 +264,7 @@ class Parameters {
static std::atomic<double> peak_sampling_heap_growth_fraction_;
static std::atomic<bool> per_cpu_caches_enabled_;
static std::atomic<bool> release_partial_alloc_pages_;
static std::atomic<bool> huge_region_demand_based_release_;
static std::atomic<bool> huge_cache_demand_based_release_;
static std::atomic<bool> release_pages_from_huge_region_;
static std::atomic<bool> resize_size_class_max_capacity_;
Expand Down
21 changes: 5 additions & 16 deletions tcmalloc/testing/get_stats_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,8 @@ TEST_F(GetStatsTest, Pbtxt) {
"tcmalloc_cache_demand_release_long_interval_ns: 300000000000"));
#endif
EXPECT_THAT(buf, HasSubstr("tcmalloc_release_partial_alloc_pages: true"));
if (Parameters::huge_region_demand_based_release()) {
EXPECT_THAT(buf,
HasSubstr("tcmalloc_huge_region_demand_based_release: true"));
} else {
EXPECT_THAT(buf,
HasSubstr("tcmalloc_huge_region_demand_based_release: false"));
}
EXPECT_THAT(buf,
HasSubstr("tcmalloc_huge_region_demand_based_release: false"));
EXPECT_THAT(buf,
HasSubstr("tcmalloc_huge_cache_demand_based_release: false"));
EXPECT_THAT(buf, HasSubstr("tcmalloc_release_pages_from_huge_region: true"));
Expand Down Expand Up @@ -186,15 +181,9 @@ TEST_F(GetStatsTest, Parameters) {
EXPECT_THAT(
buf,
HasSubstr(R"(PARAMETER tcmalloc_huge_cache_demand_based_release 0)"));
if (Parameters::huge_region_demand_based_release()) {
EXPECT_THAT(
buf, HasSubstr(
R"(PARAMETER tcmalloc_huge_region_demand_based_release 1)"));
} else {
EXPECT_THAT(
buf, HasSubstr(
R"(PARAMETER tcmalloc_huge_region_demand_based_release 0)"));
}
EXPECT_THAT(
buf,
HasSubstr(R"(PARAMETER tcmalloc_huge_region_demand_based_release 0)"));
EXPECT_THAT(
buf,
HasSubstr(R"(PARAMETER tcmalloc_release_pages_from_huge_region 1)"));
Expand Down

0 comments on commit eafcc39

Please sign in to comment.