Revert: Use recent demand to decide the number of pages to release from HugeRegion.

PiperOrigin-RevId: 827735509
Change-Id: I33df32552b13e5194f5eb0b08011574db5146cb7

Author: q-ge

tcmalloc/BUILD

@@ -975,7 +975,6 @@ cc_test(
     copts = TCMALLOC_DEFAULT_COPTS,
     deps = [
         ":common_8k_pages",
-        "//tcmalloc/internal:clock",
         "//tcmalloc/internal:logging",
         "//tcmalloc/internal:system_allocator",
         "//tcmalloc/testing:thread_manager",

tcmalloc/global_stats.cc

@@ -565,8 +565,6 @@ void DumpStats(Printer& out, int level) {
                    Parameters::filler_skip_subrelease_long_interval()));
     out.printf("PARAMETER tcmalloc_release_partial_alloc_pages %d\n",
                Parameters::release_partial_alloc_pages() ? 1 : 0);
-    out.printf("PARAMETER tcmalloc_huge_region_demand_based_release %d\n",
-               Parameters::huge_region_demand_based_release() ? 1 : 0);
     out.printf("PARAMETER tcmalloc_release_pages_from_huge_region %d\n",
                Parameters::release_pages_from_huge_region() ? 1 : 0);
     out.printf("PARAMETER tcmalloc_use_wider_slabs %d\n",
@@ -776,8 +774,6 @@ void DumpStatsInPbtxt(Printer& out, int level) {
                       Parameters::filler_skip_subrelease_long_interval()));
   region.PrintBool("tcmalloc_release_partial_alloc_pages",
                    Parameters::release_partial_alloc_pages());
-  region.PrintBool("tcmalloc_huge_region_demand_based_release",
-                   Parameters::huge_region_demand_based_release());
   region.PrintBool("tcmalloc_release_pages_from_huge_region",
                    Parameters::release_pages_from_huge_region());
   region.PrintI64("profile_sampling_interval",

tcmalloc/huge_page_aware_allocator.h

@@ -71,9 +71,6 @@ class StaticForwarder {
     return Parameters::release_partial_alloc_pages();
   }
 
-  static bool huge_region_demand_based_release() {
-    return Parameters::huge_region_demand_based_release();
-  }
 
   static bool hpaa_subrelease() { return Parameters::hpaa_subrelease(); }
 
@@ -1003,19 +1000,7 @@ inline Length HugePageAwareAllocator<Forwarder>::ReleaseAtLeastNPages(
   // the experiment is enabled. We can also explore releasing only a desired
   // number of pages.
   if (regions_.UseHugeRegionMoreOften()) {
-    if (forwarder_.huge_region_demand_based_release()) {
-      Length desired = released > num_pages ? Length(0) : num_pages - released;
-      released += regions_.ReleasePagesByPeakDemand(
-          desired,
-          SkipSubreleaseIntervals{
-              .short_interval =
-                  forwarder_.filler_skip_subrelease_short_interval(),
-              .long_interval =
-                  forwarder_.filler_skip_subrelease_long_interval()},
-          /*hit_limit*/ false);
-    } else {
-      released += regions_.ReleasePages(kFractionToReleaseFromRegion);
-    }
+    released += regions_.ReleasePages(kFractionToReleaseFromRegion);
   }
 
   // This is our long term plan but in current state will lead to insufficient
@@ -1216,13 +1201,7 @@ HugePageAwareAllocator<Forwarder>::ReleaseAtLeastNPagesBreakingHugepages(
   released += cache_.ReleaseCachedPages(HLFromPages(n)).in_pages();
 
   // We try to release as many free hugepages from HugeRegion as possible.
-  if (forwarder_.huge_region_demand_based_release()) {
-    released += regions_.ReleasePagesByPeakDemand(
-        n - released, SkipSubreleaseIntervals{}, /*hit_limit=*/true);
-  } else {
-    released += regions_.ReleasePages(/*release_fraction=*/1.0);
-  }
-
+  released += regions_.ReleasePages(/*release_fraction=*/1.0);
   if (released >= n) {
     info_.RecordRelease(n, released, reason);
     return released;

tcmalloc/huge_region.h

@@ -23,11 +23,8 @@
 #include "absl/base/internal/cycleclock.h"
 #include "absl/base/nullability.h"
 #include "absl/base/optimization.h"
-#include "absl/time/time.h"
 #include "tcmalloc/huge_cache.h"
-#include "tcmalloc/huge_page_subrelease.h"
 #include "tcmalloc/huge_pages.h"
-#include "tcmalloc/internal/clock.h"
 #include "tcmalloc/internal/config.h"
 #include "tcmalloc/internal/linked_list.h"
 #include "tcmalloc/internal/logging.h"
@@ -152,17 +149,8 @@ class HugeRegion : public TList<HugeRegion>::Elem {
 template <typename Region>
 class HugeRegionSet {
  public:
-  // For testing with mock clock.
-  HugeRegionSet(HugeRegionUsageOption use_huge_region_more_often, Clock clock)
-      : n_(0),
-        use_huge_region_more_often_(use_huge_region_more_often),
-        regionstats_tracker_(clock, absl::Minutes(10), absl::Minutes(5)) {}
-
   explicit HugeRegionSet(HugeRegionUsageOption use_huge_region_more_often)
-      : HugeRegionSet(
-            use_huge_region_more_often,
-            Clock{.now = absl::base_internal::CycleClock::Now,
-                  .freq = absl::base_internal::CycleClock::Frequency}) {}
+      : n_(0), use_huge_region_more_often_(use_huge_region_more_often) {}
 
   // If available, return a range of n free pages, setting *from_released =
   // true iff the returned range is currently unbacked.
@@ -176,19 +164,6 @@ class HugeRegionSet {
   // Add region to the set.
   void Contribute(Region* region);
 
-  // Tries to release up to <desired> number of pages from fully-free but backed
-  // hugepages in HugeRegions. <intervals> defines the skip-subrelease
-  // intervals, but unlike HugePageFiller skip-subrelease, it only releases free
-  // hugepages.
-  // Releases all free and backed hugepages to system when <hit_limit> is set to
-  // true. Else, it uses intervals to determine recent demand as seen by
-  // HugeRegions to compute realized fragmentation. It may only release as much
-  // memory in free pages as determined by the realized fragmentation.
-  // Returns the number of pages actually released.
-  Length ReleasePagesByPeakDemand(Length desired,
-                                  SkipSubreleaseIntervals intervals,
-                                  bool hit_limit);
-
   // Release hugepages that are unused but backed.
   // Releases up to <release_fraction> times number of free-but-backed hugepages
   // from each huge region. Note that this clamps release_fraction between 0 and
@@ -259,59 +234,10 @@ class HugeRegionSet {
     list_.append(r);
   }
 
-  using StatsTrackerType = SubreleaseStatsTracker<600>;
-  StatsTrackerType::SubreleaseStats GetSubreleaseStats() const {
-    StatsTrackerType::SubreleaseStats stats;
-    for (Region* region : list_) {
-      stats.num_pages += region->used_pages();
-      stats.free_pages += region->free_pages();
-      stats.unmapped_pages += region->unmapped_pages();
-      stats.huge_pages[StatsTrackerType::kRegular] += region->size();
-    }
-    stats.num_pages_subreleased = subrelease_stats_.num_pages_subreleased;
-    return stats;
-  }
-
-  Length used_pages() const {
-    Length used;
-    for (Region* region : list_) {
-      used += region->used_pages();
-    }
-    return used;
-  }
-
-  Length free_pages() const {
-    Length free;
-    for (Region* region : list_) {
-      free += region->free_pages();
-    }
-    return free;
-  }
-
-  HugeLength size() const {
-    HugeLength size;
-    for (Region* region : list_) {
-      size += region->size();
-    }
-    return size;
-  }
-
   size_t n_;
   HugeRegionUsageOption use_huge_region_more_often_;
   // Sorted by longest_free increasing.
   TList<Region> list_;
-
-  // Computes the recent demand to compute the number of pages that may be
-  // released. <desired> determines an upper-bound on the number of pages to
-  // release.
-  // Returns number of pages that may be released based on recent demand.
-  Length GetDesiredReleasablePages(Length desired,
-                                   SkipSubreleaseIntervals intervals);
-
-  // Functionality related to tracking demand.
-  void UpdateStatsTracker();
-  StatsTrackerType regionstats_tracker_;
-  SubreleaseStats subrelease_stats_;
 };
 
 // REQUIRES: r.len() == size(); r unbacked.
@@ -558,78 +484,6 @@ inline HugeLength HugeRegion::UnbackHugepages(
   return released;
 }
 
-template <typename Region>
-inline Length HugeRegionSet<Region>::GetDesiredReleasablePages(
-    Length desired, SkipSubreleaseIntervals intervals) {
-  if (!intervals.SkipSubreleaseEnabled()) {
-    return desired;
-  }
-  UpdateStatsTracker();
-
-  Length required_pages;
-  // There are two ways to calculate the demand requirement. We give priority to
-  // using the peak if peak_interval is set.
-  if (intervals.IsPeakIntervalSet()) {
-    required_pages =
-        regionstats_tracker_.GetRecentPeak(intervals.peak_interval);
-  } else {
-    required_pages = regionstats_tracker_.GetRecentDemand(
-        intervals.short_interval, intervals.long_interval);
-  }
-
-  Length current_pages = used_pages() + free_pages();
-
-  if (required_pages != Length(0)) {
-    Length new_desired;
-    if (required_pages < current_pages) {
-      new_desired = current_pages - required_pages;
-    }
-
-    // Because we currently release pages from fully backed and free hugepages,
-    // make sure that the realized fragmentation in HugeRegion is at least equal
-    // to kPagesPerHugePage. Otherwise, return zero to make sure we do not
-    // release any pages.
-    if (new_desired < kPagesPerHugePage) {
-      new_desired = Length(0);
-    }
-
-    if (new_desired >= desired) {
-      return desired;
-    }
-
-    // Compute the number of releasable pages from HugeRegion. We do not
-    // subrelease pages yet. Instead, we only release hugepages that are fully
-    // free but backed. Note: the remaining target should always be smaller or
-    // equal to the number of free pages according to the mechanism (recent peak
-    // is always larger or equal to current used_pages), however, we still
-    // calculate allowed release using the minimum of the two to avoid relying
-    // on that assumption.
-    Length free_backed_pages = free_backed().in_pages();
-    Length releasable_pages = std::min(free_backed_pages, new_desired);
-
-    // Reports the amount of memory that we didn't release due to this
-    // mechanism, but never more than skipped free pages. In other words,
-    // skipped_pages is zero if all free pages are allowed to be released by
-    // this mechanism. Note, only free pages in the smaller of the two
-    // (current_pages and required_pages) are skipped, the rest are allowed to
-    // be subreleased.
-    Length skipped_pages = std::min((free_backed_pages - releasable_pages),
-                                    (desired - new_desired));
-
-    regionstats_tracker_.ReportSkippedSubreleasePages(
-        skipped_pages, std::min(current_pages, required_pages));
-    return new_desired;
-  }
-
-  return desired;
-}
-
-template <typename Region>
-inline void HugeRegionSet<Region>::UpdateStatsTracker() {
-  regionstats_tracker_.Report(GetSubreleaseStats());
-  subrelease_stats_.reset();
-}
-
 // If available, return a range of n free pages, setting *from_released =
 // true iff the returned range is currently unbacked.
 // Returns false if no range available.
@@ -639,7 +493,6 @@ inline bool HugeRegionSet<Region>::MaybeGet(Length n, PageId* page,
   for (Region* region : list_) {
     if (region->MaybeGet(n, page, from_released)) {
       Fix(region);
-      UpdateStatsTracker();
       return true;
     }
   }
@@ -657,7 +510,6 @@ inline bool HugeRegionSet<Region>::MaybePut(Range r) {
     if (region->contains(r.p)) {
       region->Put(r, release);
       Fix(region);
-      UpdateStatsTracker();
       return true;
     }
   }
@@ -670,49 +522,6 @@ template <typename Region>
 inline void HugeRegionSet<Region>::Contribute(Region* region) {
   n_++;
   AddToList(region);
-  UpdateStatsTracker();
-}
-
-template <typename Region>
-inline Length HugeRegionSet<Region>::ReleasePagesByPeakDemand(
-    Length desired, SkipSubreleaseIntervals intervals, bool hit_limit) {
-  // Because we are releasing fully-freed hugepages, in cases when malloc
-  // release rate is set to zero, we would still want to release some pages,
-  // provided it is allowed by the demand-based release strategy. We try to
-  // release up to 10% of the free and backed hugepages.
-  if (!hit_limit && desired == Length(0)) {
-    size_t new_desired =
-        kFractionToReleaseFromRegion * free_backed().in_pages().raw_num();
-    desired = Length(new_desired);
-  }
-
-  // Only reduce desired if skip subrelease is on.
-  //
-  // Additionally, if we hit the limit, we should not be applying skip
-  // subrelease.  OOM may be imminent.
-  if (intervals.SkipSubreleaseEnabled() && !hit_limit) {
-    desired = GetDesiredReleasablePages(desired, intervals);
-  }
-
-  subrelease_stats_.set_limit_hit(hit_limit);
-
-  Length released;
-  if (desired != Length(0)) {
-    for (Region* region : list_) {
-      released += region->Release(desired - released).in_pages();
-      if (released >= desired) break;
-    }
-  }
-
-  subrelease_stats_.num_pages_subreleased += released;
-
-  // Keep separate stats if the on going release is triggered by reaching
-  // tcmalloc limit.
-  if (subrelease_stats_.limit_hit()) {
-    subrelease_stats_.total_pages_subreleased_due_to_limit += released;
-  }
-
-  return released;
 }
 
 template <typename Region>
@@ -758,17 +567,6 @@ inline void HugeRegionSet<Region>::Print(Printer& out) const {
              in_pages > Length(0) ? static_cast<double>(total_free.raw_num()) /
                                         static_cast<double>(in_pages.raw_num())
                                   : 0.0);
-
-  // Subrelease telemetry.
-  out.printf(
-      "HugeRegion: Since startup, %zu pages subreleased, %zu hugepages "
-      "broken, (%zu pages, %zu hugepages due to reaching tcmalloc limit)\n",
-      subrelease_stats_.total_pages_subreleased.raw_num(),
-      subrelease_stats_.total_hugepages_broken.raw_num(),
-      subrelease_stats_.total_pages_subreleased_due_to_limit.raw_num(),
-      subrelease_stats_.total_hugepages_broken_due_to_limit.raw_num());
-
-  regionstats_tracker_.Print(out, "HugeRegion");
 }
 
 template <typename Region>
@@ -779,17 +577,6 @@ inline void HugeRegionSet<Region>::PrintInPbtxt(PbtxtRegion& hpaa) const {
     auto detail = hpaa.CreateSubRegion("huge_region_details");
     region->PrintInPbtxt(detail);
   }
-
-  hpaa.PrintI64("region_num_pages_subreleased",
-                subrelease_stats_.total_pages_subreleased.raw_num());
-  hpaa.PrintI64(
-      "region_num_pages_subreleased_due_to_limit",
-      subrelease_stats_.total_pages_subreleased_due_to_limit.raw_num());
-
-  regionstats_tracker_.PrintSubreleaseStatsInPbtxt(hpaa,
-                                                   "region_skipped_subrelease");
-  regionstats_tracker_.PrintTimeseriesStatsInPbtxt(hpaa,
-                                                   "region_stats_timeseries");
 }
 
 template <typename Region>