diff --git a/tcmalloc/huge_page_filler.h b/tcmalloc/huge_page_filler.h index b79ab51f2..07b66dab3 100644 --- a/tcmalloc/huge_page_filler.h +++ b/tcmalloc/huge_page_filler.h @@ -1000,7 +1000,7 @@ class HugePageFiller { static constexpr size_t kLifetimeBuckets = huge_page_filler_internal::UsageInfo::kLifetimeBuckets; using LifetimeHisto = huge_page_filler_internal::UsageInfo::LifetimeHisto; - void RecordLifetime(const TrackerType* pt); + void RecordLifetime(const TrackerType* pt, double now); void PrintLifetimeHisto(Printer& out, LifetimeHisto h, AccessDensityPrediction type, absl::string_view blurb) const; @@ -1060,7 +1060,7 @@ class HugePageFiller { Length unmapping_unaccounted_; // Functionality related to time series tracking. - void UpdateFillerStatsTracker(); + void UpdateFillerStatsTracker(int64_t now); using StatsTrackerType = SubreleaseStatsTracker<600>; StatsTrackerType fillerstats_tracker_; @@ -1254,13 +1254,16 @@ HugePageFiller::TryGet(Length n, SpanAllocInfo span_alloc_info) { // We're being used for an allocation, so we are no longer considered // donated by this point. TC_ASSERT(!pt->donated()); - UpdateFillerStatsTracker(); + UpdateFillerStatsTracker(now); return {pt, page_allocation.page, was_released}; } template -void HugePageFiller::RecordLifetime(const TrackerType* pt) { - const double now = clock_.now(); +void HugePageFiller::RecordLifetime(const TrackerType* pt, + double now) { +#ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING + now = clock_.now(); +#endif const double frequency = clock_.freq(); const double elapsed = std::max(now - pt->alloctime(), 0); const absl::Duration lifetime = @@ -1313,6 +1316,11 @@ void HugePageFiller::PrintLifetimeHistoInPbtxt( template inline TrackerType* HugePageFiller::Put( TrackerType* pt, Range r, SpanAllocInfo span_alloc_info) { +#ifndef TCMALLOC_INTERNAL_LEGACY_LOCKING + const int64_t now = clock_.now(); +#else + const int64_t now = 0; +#endif RemoveFromFillerList(pt); pt->Put(r, span_alloc_info); if (pt->HasDenseSpans()) { @@ -1359,8 +1367,8 @@ inline TrackerType* HugePageFiller::Put( } if (!pt->DontFreeTracker()) { - RecordLifetime(pt); - UpdateFillerStatsTracker(); + RecordLifetime(pt, now); + UpdateFillerStatsTracker(now); if (pt->GetTagState().sampled_for_tagging) { // Set the default region name if the tracked was sampled. pt->SetAnonVmaName(set_anon_vma_name_, /*name=*/std::nullopt); @@ -1369,7 +1377,7 @@ inline TrackerType* HugePageFiller::Put( } } AddToFillerList(pt); - UpdateFillerStatsTracker(); + UpdateFillerStatsTracker(now); return nullptr; } @@ -1398,7 +1406,7 @@ inline void HugePageFiller::Contribute( } ++size_; - UpdateFillerStatsTracker(); + UpdateFillerStatsTracker(clock_.now()); } template @@ -1532,7 +1540,7 @@ inline Length HugePageFiller::GetDesiredSubreleasePages( if (!intervals.SkipSubreleaseEnabled()) { return desired; } - UpdateFillerStatsTracker(); + UpdateFillerStatsTracker(clock_.now()); Length required_pages; // As mentioned above, there are two ways to calculate the demand // requirement. We give priority to using the peak if peak_interval is set. @@ -2392,13 +2400,18 @@ inline void HugePageFiller::PrintInPbtxt( } template -inline void HugePageFiller::UpdateFillerStatsTracker() { +inline void HugePageFiller::UpdateFillerStatsTracker( + [[maybe_unused]] int64_t now) { StatsTrackerType::SubreleaseStats stats; stats.num_pages = pages_allocated(); stats.free_pages = free_pages(); stats.unmapped_pages = unmapped_pages(); stats.num_pages_subreleased = subrelease_stats_.num_pages_subreleased; - fillerstats_tracker_.Report(stats); +#ifdef TCMALLOC_INTERNAL_LEGACY_LOCKING + fillerstats_tracker_.Report(stats, clock_.now()); +#else + fillerstats_tracker_.Report(stats, now); +#endif subrelease_stats_.reset(); } diff --git a/tcmalloc/huge_page_subrelease.h b/tcmalloc/huge_page_subrelease.h index 5b2045c9f..b6e288b96 100644 --- a/tcmalloc/huge_page_subrelease.h +++ b/tcmalloc/huge_page_subrelease.h @@ -87,12 +87,12 @@ class SkippedSubreleaseCorrectnessTracker { tracker_.Report(update); } - void ReportUpdatedPeak(Length current_peak) { + void ReportUpdatedPeak(Length current_peak, int64_t now) { // Record this peak for the current epoch (so we don't double-count correct // predictions later) and advance the tracker. SkippedSubreleaseUpdate update; update.confirmed_peak = current_peak; - if (tracker_.Report(update)) { + if (tracker_.Report(update, now)) { // Also keep track of the largest peak we have confirmed this epoch. last_confirmed_peak_ = Length(0); } @@ -283,8 +283,8 @@ class SubreleaseStatsTracker { SubreleaseStatsTracker(const SubreleaseStatsTracker&) = delete; SubreleaseStatsTracker& operator=(const SubreleaseStatsTracker&) = delete; - void Report(const SubreleaseStats& stats) { - if (ABSL_PREDICT_FALSE(tracker_.Report(stats))) { + void Report(const SubreleaseStats& stats, int64_t now) { + if (ABSL_PREDICT_FALSE(tracker_.Report(stats, now))) { if (ABSL_PREDICT_FALSE(pending_skipped().count > 0)) { Length peak = stats.num_pages; // Consider the peak in the most recent record that reported within the @@ -295,7 +295,7 @@ class SubreleaseStatsTracker { peak = std::max( peak, most_recent_record.data.stats[kStatsAtMaxDemand].num_pages); } - skipped_subrelease_correctness_.ReportUpdatedPeak(peak); + skipped_subrelease_correctness_.ReportUpdatedPeak(peak, now); } } } diff --git a/tcmalloc/huge_page_subrelease_test.cc b/tcmalloc/huge_page_subrelease_test.cc index 3be118689..cf3f7785a 100644 --- a/tcmalloc/huge_page_subrelease_test.cc +++ b/tcmalloc/huge_page_subrelease_test.cc @@ -92,7 +92,7 @@ void StatsTrackerTest::GenerateInterestingPoints(Length num_pages, stats.num_pages = num_pages + Length((i == 0) ? 4 : 8 * j); stats.free_pages = num_free_pages + Length(10 * i + j); stats.unmapped_pages = Length(10); - tracker_.Report(stats); + tracker_.Report(stats, clock_); } } } @@ -103,7 +103,7 @@ void StatsTrackerTest::GenerateDemandPoint(Length num_pages, stats.num_pages = num_pages; stats.free_pages = num_free_pages; stats.unmapped_pages = Length(0); - tracker_.Report(stats); + tracker_.Report(stats, clock_); } // Tests that the tracker aggregates all data correctly. The output is tested by diff --git a/tcmalloc/internal/timeseries_tracker.h b/tcmalloc/internal/timeseries_tracker.h index fb30c1194..b078f099d 100644 --- a/tcmalloc/internal/timeseries_tracker.h +++ b/tcmalloc/internal/timeseries_tracker.h @@ -64,7 +64,8 @@ class TimeSeriesTracker { InitTracker(); } - bool Report(const S& val); + bool Report(const S& val, int64_t now); + bool Report(const S& val) { return Report(val, clock_.now()); } // Iterates over the time series, starting from the oldest entry. The callback // receives the sequence number of the entry, the epoch_delta (i.e., number of @@ -92,40 +93,28 @@ class TimeSeriesTracker { private: // Returns true if the tracker moved to a different epoch. - bool UpdateClock(); + bool UpdateClock(int64_t now); + bool UpdateClock() { return UpdateClock(clock_.now()); } // Returns the current epoch number based on the clock. - int64_t GetCurrentEpoch() { - // This is equivalent to - // `clock_.now() / (absl::ToDoubleSeconds(epoch_length_) * clock_.freq())`. - // We basically follow the technique from - // https://ridiculousfish.com/blog/posts/labor-of-division-episode-i.html, - // except that we use one fewer bit of precision than necessary to always - // get the correct answer if the numerator were a 64-bit unsigned number. In - // this case, because clock_.now() returns a signed 64-bit number (i.e. max - // is <2^63), it shouldn't cause a problem. This way, we don't need to - // handle overflow so it's simpler. See also: - // https://lemire.me/blog/2019/02/20/more-fun-with-fast-remainders-when-the-divisor-is-a-constant/. + int64_t GetCurrentEpoch(int64_t now) const { return static_cast(static_cast(epoch_ticks_m_) * - clock_.now() >> + std::max(0, now) >> div_precision_); } - void InitTracker() { - // Inits the tracker by "create" an record for "now" on slot 0. The record - // serves as the first valid record in the tracker, with epoch coverage - // (delta) 1 and an empty payload. In this way, we would know when was the - // first real data point taken, as it would be the time diff between "now" - // and "then". If the first data point is taken immediately, this record - // will be updated with real data. + int64_t GetCurrentEpoch() const { return GetCurrentEpoch(clock_.now()); } + + void InitTracker(int64_t now) { current_slot_ = 0; for (auto& entry : entries_) { entry = TimeSeriesContent(); } size_t delta = 1; - last_epoch_ = GetCurrentEpoch(); + last_epoch_ = GetCurrentEpoch(now); entries_[current_slot_] = TimeSeriesContent(delta); covered_epochs_ = delta; } + void InitTracker() { InitTracker(clock_.now()); } struct TimeSeriesContent { size_t epoch_delta; // Time gap (in epoch) between this slot and @@ -155,14 +144,14 @@ class TimeSeriesTracker { // Advances the current slot if the clock had advanced >= 1 epoch; sets the // epoch_delta for how many epoch had passed sice the previous clock update. template -bool TimeSeriesTracker::UpdateClock() { - const size_t epoch = GetCurrentEpoch(); +bool TimeSeriesTracker::UpdateClock(int64_t now) { + const size_t epoch = GetCurrentEpoch(now); if (ABSL_PREDICT_FALSE(epoch < last_epoch_)) { // If the clock has regressed (e.g., across snapshot restore or container // migration to a host with a lower monotonic clock), reset the tracker to // prevent unsigned underflow in delta calculations and discard stale // history. - InitTracker(); + InitTracker(now); return false; } if (epoch == last_epoch_) { @@ -243,8 +232,8 @@ TimeSeriesTracker::GetMostRecentRecord() const { } template -bool TimeSeriesTracker::Report(const S& val) { - bool updated_clock = UpdateClock(); +bool TimeSeriesTracker::Report(const S& val, int64_t now) { + bool updated_clock = UpdateClock(now); entries_[current_slot_].payload.Report(val); return updated_clock; } diff --git a/tcmalloc/internal/timeseries_tracker_test.cc b/tcmalloc/internal/timeseries_tracker_test.cc index 5479e0805..8d6252ff8 100644 --- a/tcmalloc/internal/timeseries_tracker_test.cc +++ b/tcmalloc/internal/timeseries_tracker_test.cc @@ -345,6 +345,31 @@ TEST_F(TimeSeriesTrackerTest, ClockRegression) { EXPECT_THAT(recent_record.data.values_, ElementsAre(4)); } +TEST_F(TimeSeriesTrackerTest, ExplicitTimestamp) { + const int64_t t0 = 0; + tracker_.Report(1, t0); + const int64_t t1 = absl::ToDoubleNanoseconds(absl::Seconds(1)); + tracker_.Report(2, t1); + + std::vector> all_values; + tracker_.Iter([&](size_t offset, size_t epoch_delta, const TestEntry& e) { + all_values.push_back(e.values_); + }); + EXPECT_THAT(all_values, ElementsAre(ElementsAre(1), ElementsAre(2))); +} + +TEST_F(TimeSeriesTrackerTest, ClampedNegativeClock) { + tracker_.Report(1, -1000); + tracker_.Report(2, -1); + tracker_.Report(3, 0); + + std::vector> all_values; + tracker_.Iter([&](size_t offset, size_t epoch_delta, const TestEntry& e) { + all_values.push_back(e.values_); + }); + EXPECT_THAT(all_values, ElementsAre(ElementsAre(1, 2, 3))); +} + } // namespace } // namespace tcmalloc_internal } // namespace tcmalloc