diff options
author | arkady-e1ppa <arkady-e1ppa@yandex-team.com> | 2024-10-24 19:03:01 +0300 |
---|---|---|
committer | arkady-e1ppa <arkady-e1ppa@yandex-team.com> | 2024-10-24 19:15:34 +0300 |
commit | 72ac69bdd5bbfeb6a8ee1eb073cdbcc94193b33e (patch) | |
tree | 266db0f4f4a398abd8279ce2476d62818b84f04c | |
parent | db3e5f504d17fb1ccb5baa7cc4ca79c63dde159c (diff) | |
download | ydb-72ac69bdd5bbfeb6a8ee1eb073cdbcc94193b33e.tar.gz |
YT-20193: Try using WeakPtr in profiler fields
SelfProfilers in exporter, producers and sensor sets used to hold strong ref to registry which has said classes as fields forming a reference cycle. Usage of weak pointers in these profilers fixes said problem. We don’t convert to weak profilers everywhere since, sadly, some external users have adopted the usage of profilers as strong ref holders
commit_hash:26d6f1200293a66faa5df9b1117b7becc63f59d8
-rw-r--r-- | yt/yt/library/profiling/sensor.cpp | 268 | ||||
-rw-r--r-- | yt/yt/library/profiling/sensor.h | 85 | ||||
-rw-r--r-- | yt/yt/library/profiling/solomon/exporter.cpp | 2 | ||||
-rw-r--r-- | yt/yt/library/profiling/solomon/producer.cpp | 2 | ||||
-rw-r--r-- | yt/yt/library/profiling/solomon/producer.h | 4 | ||||
-rw-r--r-- | yt/yt/library/profiling/solomon/registry.cpp | 4 | ||||
-rw-r--r-- | yt/yt/library/profiling/solomon/registry.h | 6 | ||||
-rw-r--r-- | yt/yt/library/profiling/solomon/sensor_set.cpp | 2 | ||||
-rw-r--r-- | yt/yt/library/profiling/solomon/sensor_set.h | 2 | ||||
-rw-r--r-- | yt/yt/library/profiling/unittests/exporter_ut.cpp | 13 |
10 files changed, 267 insertions, 121 deletions
diff --git a/yt/yt/library/profiling/sensor.cpp b/yt/yt/library/profiling/sensor.cpp index f04791b3cf..f795a01c91 100644 --- a/yt/yt/library/profiling/sensor.cpp +++ b/yt/yt/library/profiling/sensor.cpp @@ -259,40 +259,77 @@ bool TSensorOptions::IsCompatibleWith(const TSensorOptions& other) const //////////////////////////////////////////////////////////////////////////////// -TProfiler::TProfiler( +namespace NDetail { + +template <bool UseWeakPtr> +TRegistryHolderBase<UseWeakPtr>::TRegistryHolderBase(const IRegistryImplPtr& impl) + : Impl_(impl) +{ } + +template <bool UseWeakPtr> +const IRegistryImplPtr& TRegistryHolderBase<UseWeakPtr>::GetRegistry() const +{ + return Impl_; +} + +//////////////////////////////////////////////////////////////////////////////// + +TRegistryHolderBase<true>::TRegistryHolderBase(const IRegistryImplPtr& impl) + : Impl_(impl) +{ } + +IRegistryImplPtr TRegistryHolderBase<true>::GetRegistry() const +{ + return Impl_.Lock(); +} + +// NB(arkady-e1ppa): Explicit template instantiation somehow does not require +// base classes to be instantiated therefore we must do that by hand separately. +// template class TRegistryHolderBase<true>; +// ^However, full specialization is +// treated differently and is pointless and not required == banned by clang :). +template class TRegistryHolderBase<false>; + +//////////////////////////////////////////////////////////////////////////////// + +template <bool UseWeakPtr> +TProfiler<UseWeakPtr>::TProfiler( const IRegistryImplPtr& impl, const std::string& prefix, const std::string& _namespace) - : Enabled_(true) + : TBase(impl) + , Enabled_(true) , Prefix_(prefix) , Namespace_(_namespace) - , Impl_(impl) { } -TProfiler::TProfiler( +template <bool UseWeakPtr> +TProfiler<UseWeakPtr>::TProfiler( const std::string& prefix, const std::string& _namespace, const TTagSet& tags, const IRegistryImplPtr& impl, TSensorOptions options) - : Enabled_(true) + : TBase(impl ? impl : GetGlobalRegistry()) + , Enabled_(true) , Prefix_(prefix) , Namespace_(_namespace) , Tags_(tags) , Options_(options) - , Impl_(impl ? impl : GetGlobalRegistry()) { } -TProfiler TProfiler::WithPrefix(const std::string& prefix) const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithPrefix(const std::string& prefix) const { if (!Enabled_) { return {}; } - return TProfiler(Prefix_ + prefix, Namespace_, Tags_, Impl_, Options_); + return TProfiler<UseWeakPtr>(Prefix_ + prefix, Namespace_, Tags_, GetRegistry(), Options_); } -TProfiler TProfiler::WithTag(const std::string& name, const std::string& value, int parent) const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithTag(const std::string& name, const std::string& value, int parent) const { if (!Enabled_) { return {}; @@ -300,19 +337,19 @@ TProfiler TProfiler::WithTag(const std::string& name, const std::string& value, auto allTags = Tags_; allTags.AddTag(std::pair(name, value), parent); - return TProfiler(Prefix_, Namespace_, allTags, Impl_, Options_); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, allTags, GetRegistry(), Options_); } -void TProfiler::RenameDynamicTag(const TDynamicTagPtr& tag, const std::string& name, const std::string& value) const +template <bool UseWeakPtr> +void TProfiler<UseWeakPtr>::RenameDynamicTag(const TDynamicTagPtr& tag, const std::string& name, const std::string& value) const { - if (!Impl_) { - return; + if (const auto& impl = GetRegistry()) { + impl->RenameDynamicTag(tag, name, value); } - - Impl_->RenameDynamicTag(tag, name, value); } -TProfiler TProfiler::WithRequiredTag(const std::string& name, const std::string& value, int parent) const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithRequiredTag(const std::string& name, const std::string& value, int parent) const { if (!Enabled_) { return {}; @@ -320,10 +357,11 @@ TProfiler TProfiler::WithRequiredTag(const std::string& name, const std::string& auto allTags = Tags_; allTags.AddRequiredTag(std::pair(name, value), parent); - return TProfiler(Prefix_, Namespace_, allTags, Impl_, Options_); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, allTags, GetRegistry(), Options_); } -TProfiler TProfiler::WithExcludedTag(const std::string& name, const std::string& value, int parent) const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithExcludedTag(const std::string& name, const std::string& value, int parent) const { if (!Enabled_) { return {}; @@ -331,10 +369,11 @@ TProfiler TProfiler::WithExcludedTag(const std::string& name, const std::string& auto allTags = Tags_; allTags.AddExcludedTag(std::pair(name, value), parent); - return TProfiler(Prefix_, Namespace_, allTags, Impl_, Options_); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, allTags, GetRegistry(), Options_); } -TProfiler TProfiler::WithAlternativeTag(const std::string& name, const std::string& value, int alternativeTo, int parent) const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithAlternativeTag(const std::string& name, const std::string& value, int alternativeTo, int parent) const { if (!Enabled_) { return {}; @@ -343,10 +382,11 @@ TProfiler TProfiler::WithAlternativeTag(const std::string& name, const std::stri auto allTags = Tags_; allTags.AddAlternativeTag(std::pair(name, value), alternativeTo, parent); - return TProfiler(Prefix_, Namespace_, allTags, Impl_, Options_); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, allTags, GetRegistry(), Options_); } -TProfiler TProfiler::WithExtensionTag(const std::string& name, const std::string& value, int extensionOf) const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithExtensionTag(const std::string& name, const std::string& value, int extensionOf) const { if (!Enabled_) { return {}; @@ -355,10 +395,11 @@ TProfiler TProfiler::WithExtensionTag(const std::string& name, const std::string auto allTags = Tags_; allTags.AddExtensionTag(std::pair(name, value), extensionOf); - return TProfiler(Prefix_, Namespace_, allTags, Impl_, Options_); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, allTags, GetRegistry(), Options_); } -TProfiler TProfiler::WithTags(const TTagSet& tags) const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithTags(const TTagSet& tags) const { if (!Enabled_) { return {}; @@ -366,10 +407,11 @@ TProfiler TProfiler::WithTags(const TTagSet& tags) const auto allTags = Tags_; allTags.Append(tags); - return TProfiler(Prefix_, Namespace_, allTags, Impl_, Options_); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, allTags, GetRegistry(), Options_); } -TProfiler TProfiler::WithSparse() const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithSparse() const { if (!Enabled_) { return {}; @@ -377,10 +419,11 @@ TProfiler TProfiler::WithSparse() const auto opts = Options_; opts.Sparse = true; - return TProfiler(Prefix_, Namespace_, Tags_, Impl_, opts); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, Tags_, GetRegistry(), opts); } -TProfiler TProfiler::WithDense() const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithDense() const { if (!Enabled_) { return {}; @@ -388,10 +431,11 @@ TProfiler TProfiler::WithDense() const auto opts = Options_; opts.Sparse = false; - return TProfiler(Prefix_, Namespace_, Tags_, Impl_, opts); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, Tags_, GetRegistry(), opts); } -TProfiler TProfiler::WithGlobal() const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithGlobal() const { if (!Enabled_) { return {}; @@ -399,10 +443,11 @@ TProfiler TProfiler::WithGlobal() const auto opts = Options_; opts.Global = true; - return TProfiler(Prefix_, Namespace_, Tags_, Impl_, opts); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, Tags_, GetRegistry(), opts); } -TProfiler TProfiler::WithDefaultDisabled() const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithDefaultDisabled() const { if (!Enabled_) { return {}; @@ -410,10 +455,11 @@ TProfiler TProfiler::WithDefaultDisabled() const auto opts = Options_; opts.DisableDefault = true; - return TProfiler(Prefix_, Namespace_, Tags_, Impl_, opts); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, Tags_, GetRegistry(), opts); } -TProfiler TProfiler::WithProjectionsDisabled() const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithProjectionsDisabled() const { if (!Enabled_) { return {}; @@ -425,10 +471,11 @@ TProfiler TProfiler::WithProjectionsDisabled() const auto opts = Options_; opts.DisableProjections = true; - return TProfiler(Prefix_, Namespace_, allTags, Impl_, opts); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, allTags, GetRegistry(), opts); } -TProfiler TProfiler::WithRenameDisabled() const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithRenameDisabled() const { if (!Enabled_) { return {}; @@ -436,10 +483,11 @@ TProfiler TProfiler::WithRenameDisabled() const auto opts = Options_; opts.DisableSensorsRename = true; - return TProfiler(Prefix_, Namespace_, Tags_, Impl_, opts); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, Tags_, GetRegistry(), opts); } -TProfiler TProfiler::WithProducerRemoveSupport() const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithProducerRemoveSupport() const { if (!Enabled_) { return {}; @@ -447,10 +495,11 @@ TProfiler TProfiler::WithProducerRemoveSupport() const auto opts = Options_; opts.ProducerRemoveSupport = true; - return TProfiler(Prefix_, Namespace_, Tags_, Impl_, opts); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, Tags_, GetRegistry(), opts); } -TProfiler TProfiler::WithHot(bool value) const +template <bool UseWeakPtr> +TProfiler<UseWeakPtr> TProfiler<UseWeakPtr>::WithHot(bool value) const { if (!Enabled_) { return {}; @@ -458,56 +507,66 @@ TProfiler TProfiler::WithHot(bool value) const auto opts = Options_; opts.Hot = value; - return TProfiler(Prefix_, Namespace_, Tags_, Impl_, opts); + return TProfiler<UseWeakPtr>(Prefix_, Namespace_, Tags_, GetRegistry(), opts); } -TCounter TProfiler::Counter(const std::string& name) const +template <bool UseWeakPtr> +TCounter TProfiler<UseWeakPtr>::Counter(const std::string& name) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } TCounter counter; - counter.Counter_ = Impl_->RegisterCounter(Namespace_ + Prefix_ + name, Tags_, Options_); + counter.Counter_ = impl->RegisterCounter(Namespace_ + Prefix_ + name, Tags_, Options_); return counter; } -TTimeCounter TProfiler::TimeCounter(const std::string& name) const +template <bool UseWeakPtr> +TTimeCounter TProfiler<UseWeakPtr>::TimeCounter(const std::string& name) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } TTimeCounter counter; - counter.Counter_ = Impl_->RegisterTimeCounter(Namespace_ + Prefix_ + name, Tags_, Options_); + counter.Counter_ = impl->RegisterTimeCounter(Namespace_ + Prefix_ + name, Tags_, Options_); return counter; } -TGauge TProfiler::Gauge(const std::string& name) const +template <bool UseWeakPtr> +TGauge TProfiler<UseWeakPtr>::Gauge(const std::string& name) const { - if (!Impl_) { - return TGauge(); + const auto& impl = GetRegistry(); + if (!impl) { + return {}; } TGauge gauge; - gauge.Gauge_ = Impl_->RegisterGauge(Namespace_ + Prefix_ + name, Tags_, Options_); + gauge.Gauge_ = impl->RegisterGauge(Namespace_ + Prefix_ + name, Tags_, Options_); return gauge; } -TTimeGauge TProfiler::TimeGauge(const std::string& name) const +template <bool UseWeakPtr> +TTimeGauge TProfiler<UseWeakPtr>::TimeGauge(const std::string& name) const { - if (!Impl_) { - return TTimeGauge(); + const auto& impl = GetRegistry(); + if (!impl) { + return {}; } TTimeGauge gauge; - gauge.Gauge_ = Impl_->RegisterTimeGauge(Namespace_ + Prefix_ + name, Tags_, Options_); + gauge.Gauge_ = impl->RegisterTimeGauge(Namespace_ + Prefix_ + name, Tags_, Options_); return gauge; } -TSummary TProfiler::Summary(const std::string& name, ESummaryPolicy summaryPolicy) const +template <bool UseWeakPtr> +TSummary TProfiler<UseWeakPtr>::Summary(const std::string& name, ESummaryPolicy summaryPolicy) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } @@ -515,13 +574,15 @@ TSummary TProfiler::Summary(const std::string& name, ESummaryPolicy summaryPolic options.SummaryPolicy = summaryPolicy; TSummary summary; - summary.Summary_ = Impl_->RegisterSummary(Namespace_ + Prefix_ + name, Tags_, std::move(options)); + summary.Summary_ = impl->RegisterSummary(Namespace_ + Prefix_ + name, Tags_, std::move(options)); return summary; } -TGauge TProfiler::GaugeSummary(const std::string& name, ESummaryPolicy summaryPolicy) const +template <bool UseWeakPtr> +TGauge TProfiler<UseWeakPtr>::GaugeSummary(const std::string& name, ESummaryPolicy summaryPolicy) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } @@ -529,13 +590,15 @@ TGauge TProfiler::GaugeSummary(const std::string& name, ESummaryPolicy summaryPo options.SummaryPolicy = summaryPolicy; TGauge gauge; - gauge.Gauge_ = Impl_->RegisterGaugeSummary(Namespace_ + Prefix_ + name, Tags_, std::move(options)); + gauge.Gauge_ = impl->RegisterGaugeSummary(Namespace_ + Prefix_ + name, Tags_, std::move(options)); return gauge; } -TTimeGauge TProfiler::TimeGaugeSummary(const std::string& name, ESummaryPolicy summaryPolicy) const +template <bool UseWeakPtr> +TTimeGauge TProfiler<UseWeakPtr>::TimeGaugeSummary(const std::string& name, ESummaryPolicy summaryPolicy) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } @@ -543,24 +606,28 @@ TTimeGauge TProfiler::TimeGaugeSummary(const std::string& name, ESummaryPolicy s options.SummaryPolicy = summaryPolicy; TTimeGauge gauge; - gauge.Gauge_ = Impl_->RegisterTimeGaugeSummary(Namespace_ + Prefix_ + name, Tags_, std::move(options)); + gauge.Gauge_ = impl->RegisterTimeGaugeSummary(Namespace_ + Prefix_ + name, Tags_, std::move(options)); return gauge; } -TEventTimer TProfiler::Timer(const std::string& name) const +template <bool UseWeakPtr> +TEventTimer TProfiler<UseWeakPtr>::Timer(const std::string& name) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } TEventTimer timer; - timer.Timer_ = Impl_->RegisterTimerSummary(Namespace_ + Prefix_ + name, Tags_, Options_); + timer.Timer_ = impl->RegisterTimerSummary(Namespace_ + Prefix_ + name, Tags_, Options_); return timer; } -TEventTimer TProfiler::TimeHistogram(const std::string& name, TDuration min, TDuration max) const +template <bool UseWeakPtr> +TEventTimer TProfiler<UseWeakPtr>::TimeHistogram(const std::string& name, TDuration min, TDuration max) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } @@ -569,88 +636,93 @@ TEventTimer TProfiler::TimeHistogram(const std::string& name, TDuration min, TDu options.HistogramMax = max; TEventTimer timer; - timer.Timer_ = Impl_->RegisterTimeHistogram(Namespace_ + Prefix_ + name, Tags_, options); + timer.Timer_ = impl->RegisterTimeHistogram(Namespace_ + Prefix_ + name, Tags_, options); return timer; } -TEventTimer TProfiler::TimeHistogram(const std::string& name, std::vector<TDuration> bounds) const +template <bool UseWeakPtr> +TEventTimer TProfiler<UseWeakPtr>::TimeHistogram(const std::string& name, std::vector<TDuration> bounds) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } TEventTimer timer; auto options = Options_; options.TimeHistogramBounds = std::move(bounds); - timer.Timer_ = Impl_->RegisterTimeHistogram(Namespace_ + Prefix_ + name, Tags_, options); + timer.Timer_ = impl->RegisterTimeHistogram(Namespace_ + Prefix_ + name, Tags_, options); return timer; } -TGaugeHistogram TProfiler::GaugeHistogram(const std::string& name, std::vector<double> buckets) const +template <bool UseWeakPtr> +TGaugeHistogram TProfiler<UseWeakPtr>::GaugeHistogram(const std::string& name, std::vector<double> buckets) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } TGaugeHistogram histogram; auto options = Options_; options.HistogramBounds = std::move(buckets); - histogram.Histogram_ = Impl_->RegisterGaugeHistogram(Namespace_ + Prefix_ + name, Tags_, options); + histogram.Histogram_ = impl->RegisterGaugeHistogram(Namespace_ + Prefix_ + name, Tags_, options); return histogram; } -TRateHistogram TProfiler::RateHistogram(const std::string& name, std::vector<double> buckets) const +template <bool UseWeakPtr> +TRateHistogram TProfiler<UseWeakPtr>::RateHistogram(const std::string& name, std::vector<double> buckets) const { - if (!Impl_) { + const auto& impl = GetRegistry(); + if (!impl) { return {}; } TRateHistogram histogram; auto options = Options_; options.HistogramBounds = std::move(buckets); - histogram.Histogram_ = Impl_->RegisterRateHistogram(Namespace_ + Prefix_ + name, Tags_, options); + histogram.Histogram_ = impl->RegisterRateHistogram(Namespace_ + Prefix_ + name, Tags_, options); return histogram; } -void TProfiler::AddFuncCounter( +template <bool UseWeakPtr> +void TProfiler<UseWeakPtr>::AddFuncCounter( const std::string& name, const TRefCountedPtr& owner, std::function<i64()> reader) const { - if (!Impl_) { - return; + if (const auto& impl = GetRegistry()) { + impl->RegisterFuncCounter(Namespace_ + Prefix_ + name, Tags_, Options_, owner, reader); } - - Impl_->RegisterFuncCounter(Namespace_ + Prefix_ + name, Tags_, Options_, owner, reader); } -void TProfiler::AddFuncGauge( +template <bool UseWeakPtr> +void TProfiler<UseWeakPtr>::AddFuncGauge( const std::string& name, const TRefCountedPtr& owner, std::function<double()> reader) const { - if (!Impl_) { - return; + if (const auto& impl = GetRegistry()) { + impl->RegisterFuncGauge(Namespace_ + Prefix_ + name, Tags_, Options_, owner, reader); } - - Impl_->RegisterFuncGauge(Namespace_ + Prefix_ + name, Tags_, Options_, owner, reader); } -void TProfiler::AddProducer( +template <bool UseWeakPtr> +void TProfiler<UseWeakPtr>::AddProducer( const std::string& prefix, const ISensorProducerPtr& producer) const { - if (!Impl_) { - return; + if (const auto& impl = GetRegistry()) { + impl->RegisterProducer(Namespace_ + Prefix_ + prefix, Tags_, Options_, producer); } - - Impl_->RegisterProducer(Namespace_ + Prefix_ + prefix, Tags_, Options_, producer); } -const IRegistryImplPtr& TProfiler::GetRegistry() const -{ - return Impl_; -} +} // namespace NDetail + +//////////////////////////////////////////////////////////////////////////////// + +template class NDetail::TProfiler<true>; +template class NDetail::TProfiler<false>; //////////////////////////////////////////////////////////////////////////////// diff --git a/yt/yt/library/profiling/sensor.h b/yt/yt/library/profiling/sensor.h index 7a2f32fe10..c1cfa9e4c4 100644 --- a/yt/yt/library/profiling/sensor.h +++ b/yt/yt/library/profiling/sensor.h @@ -23,6 +23,19 @@ namespace NYT::NProfiling { //////////////////////////////////////////////////////////////////////////////// +struct TTesting; + +//////////////////////////////////////////////////////////////////////////////// + +namespace NDetail { + +template <bool> +class TProfiler; + +} // namespace NDetail + +//////////////////////////////////////////////////////////////////////////////// + class TCounter { public: @@ -35,7 +48,8 @@ public: explicit operator bool() const; private: - friend class TProfiler; + template <bool> + friend class NDetail::TProfiler; friend struct TTesting; ICounterImplPtr Counter_; @@ -51,7 +65,8 @@ public: explicit operator bool() const; private: - friend class TProfiler; + template <bool> + friend class NDetail::TProfiler; friend struct TTesting; ITimeCounterImplPtr Counter_; @@ -67,7 +82,8 @@ public: explicit operator bool() const; private: - friend class TProfiler; + template <bool> + friend class NDetail::TProfiler; friend struct TTesting; IGaugeImplPtr Gauge_; @@ -83,7 +99,8 @@ public: explicit operator bool() const; private: - friend class TProfiler; + template <bool> + friend class NDetail::TProfiler; friend struct TTesting; ITimeGaugeImplPtr Gauge_; @@ -99,7 +116,8 @@ public: explicit operator bool() const; private: - friend class TProfiler; + template <bool> + friend class NDetail::TProfiler; ISummaryImplPtr Summary_; }; @@ -114,7 +132,8 @@ public: explicit operator bool() const; private: - friend class TProfiler; + template <bool> + friend class NDetail::TProfiler; ITimerImplPtr Timer_; }; @@ -152,7 +171,8 @@ public: explicit operator bool() const; private: - friend class TProfiler; + template <bool> + friend class NDetail::TProfiler; IHistogramImplPtr Histogram_; }; @@ -167,7 +187,8 @@ public: explicit operator bool() const; private: - friend class TProfiler; + template <bool> + friend class NDetail::TProfiler; friend struct TTesting; IHistogramImplPtr Histogram_; @@ -225,9 +246,43 @@ void FormatValue(TStringBuilderBase* builder, const TSensorOptions& options, TSt //////////////////////////////////////////////////////////////////////////////// -//! TProfiler stores common settings of profiling counters. +namespace NDetail { + +template <bool UseWeakPtr> +class TRegistryHolderBase +{ +public: + explicit TRegistryHolderBase(const IRegistryImplPtr& impl = nullptr); + + const IRegistryImplPtr& GetRegistry() const; + +private: + IRegistryImplPtr Impl_; +}; + +//////////////////////////////////////////////////////////////////////////////// + +template <> +class TRegistryHolderBase<true> +{ +public: + explicit TRegistryHolderBase(const IRegistryImplPtr& impl = nullptr); + + IRegistryImplPtr GetRegistry() const; + +private: + TWeakPtr<IRegistryImpl> Impl_; +}; + +//////////////////////////////////////////////////////////////////////////////// + +template <bool UseWeakPtr> class TProfiler + : private TRegistryHolderBase<UseWeakPtr> { +private: + using TBase = TRegistryHolderBase<UseWeakPtr>; + public: //! Default constructor creates null registry. Every method of null registry is no-op. /*! @@ -409,19 +464,25 @@ public: const std::string& prefix, const ISensorProducerPtr& producer) const; - const IRegistryImplPtr& GetRegistry() const; + using TBase::GetRegistry; private: - friend struct TTesting; + friend struct ::NYT::NProfiling::TTesting; bool Enabled_ = false; std::string Prefix_; std::string Namespace_; TTagSet Tags_; TSensorOptions Options_; - IRegistryImplPtr Impl_; }; +} // namespace NDetail + +//////////////////////////////////////////////////////////////////////////////// + +using TProfiler = NDetail::TProfiler</*UseWeakPtr*/false>; +using TWeakProfiler = NDetail::TProfiler</*UseWeakPtr*/true>; + using TRegistry = TProfiler; //////////////////////////////////////////////////////////////////////////////// diff --git a/yt/yt/library/profiling/solomon/exporter.cpp b/yt/yt/library/profiling/solomon/exporter.cpp index 7f4469053b..140cf39264 100644 --- a/yt/yt/library/profiling/solomon/exporter.cpp +++ b/yt/yt/library/profiling/solomon/exporter.cpp @@ -209,7 +209,7 @@ TSolomonExporter::TSolomonExporter( {.PollingPeriod = Config_->EncodingThreadPoolPollingPeriod})) { if (Config_->EnableSelfProfiling) { - Registry_->Profile(TProfiler{Registry_, ""}); + Registry_->Profile(TWeakProfiler{Registry_, ""}); } CollectionStartDelay_ = Registry_->GetSelfProfiler().Timer("/collection_delay"); diff --git a/yt/yt/library/profiling/solomon/producer.cpp b/yt/yt/library/profiling/solomon/producer.cpp index 396ed7b36c..7e52b95fdb 100644 --- a/yt/yt/library/profiling/solomon/producer.cpp +++ b/yt/yt/library/profiling/solomon/producer.cpp @@ -251,7 +251,7 @@ void TProducerSet::Collect(IRegistryImplPtr profiler, IInvokerPtr invoker) } } -void TProducerSet::Profile(const TProfiler& profiler) +void TProducerSet::Profile(const TWeakProfiler& profiler) { SelfProfiler_ = profiler; ProducerCollectDuration_ = profiler.Timer("/producer_collect_duration"); diff --git a/yt/yt/library/profiling/solomon/producer.h b/yt/yt/library/profiling/solomon/producer.h index 1787dbe862..a6db1f1ff7 100644 --- a/yt/yt/library/profiling/solomon/producer.h +++ b/yt/yt/library/profiling/solomon/producer.h @@ -82,14 +82,14 @@ public: void Collect(IRegistryImplPtr profiler, IInvokerPtr invoker); - void Profile(const TProfiler& profiler); + void Profile(const TWeakProfiler& profiler); void SetCollectionBatchSize(int batchSize); private: THashSet<TProducerStatePtr> Producers_; - TProfiler SelfProfiler_; + TWeakProfiler SelfProfiler_; TEventTimer ProducerCollectDuration_; int BatchSize_ = DefaultProducerCollectionBatchSize; diff --git a/yt/yt/library/profiling/solomon/registry.cpp b/yt/yt/library/profiling/solomon/registry.cpp index ceaa9c17d3..0a11ddd82c 100644 --- a/yt/yt/library/profiling/solomon/registry.cpp +++ b/yt/yt/library/profiling/solomon/registry.cpp @@ -294,7 +294,7 @@ int TSolomonRegistry::IndexOf(i64 iteration) const return iteration % GetWindowSize(); } -void TSolomonRegistry::Profile(const TProfiler& profiler) +void TSolomonRegistry::Profile(const TWeakProfiler& profiler) { SelfProfiler_ = profiler.WithPrefix("/solomon_registry"); @@ -308,7 +308,7 @@ void TSolomonRegistry::Profile(const TProfiler& profiler) RegistrationCount_ = SelfProfiler_.Counter("/registration_count"); } -const TProfiler& TSolomonRegistry::GetSelfProfiler() const +const TWeakProfiler& TSolomonRegistry::GetSelfProfiler() const { return SelfProfiler_; } diff --git a/yt/yt/library/profiling/solomon/registry.h b/yt/yt/library/profiling/solomon/registry.h index 70e082a463..af99e8c2a7 100644 --- a/yt/yt/library/profiling/solomon/registry.h +++ b/yt/yt/library/profiling/solomon/registry.h @@ -149,8 +149,8 @@ public: int GetWindowSize() const; int IndexOf(i64 iteration) const; - void Profile(const TProfiler& profiler); - const TProfiler& GetSelfProfiler() const; + void Profile(const TWeakProfiler& profiler); + const TWeakProfiler& GetSelfProfiler() const; NProto::TSensorDump DumpSensors(); NProto::TSensorDump DumpSensors(std::vector<TTagId> extraTags); @@ -160,7 +160,7 @@ private: i64 Iteration_ = 0; std::optional<int> WindowSize_; std::function<int(const std::string&)> GridFactor_; - TProfiler SelfProfiler_; + TWeakProfiler SelfProfiler_; YT_DECLARE_SPIN_LOCK(NThreading::TSpinLock, DynamicTagsLock_); std::vector<TTag> DynamicTags_; diff --git a/yt/yt/library/profiling/solomon/sensor_set.cpp b/yt/yt/library/profiling/solomon/sensor_set.cpp index d64ce4527c..4368cfeba3 100644 --- a/yt/yt/library/profiling/solomon/sensor_set.cpp +++ b/yt/yt/library/profiling/solomon/sensor_set.cpp @@ -45,7 +45,7 @@ bool TSensorSet::IsEmpty() const RateHistograms_.empty(); } -void TSensorSet::Profile(const TProfiler &profiler) +void TSensorSet::Profile(const TWeakProfiler& profiler) { CubeSize_ = profiler.Gauge("/cube_size"); SensorsEmitted_ = profiler.Gauge("/sensors_emitted"); diff --git a/yt/yt/library/profiling/solomon/sensor_set.h b/yt/yt/library/profiling/solomon/sensor_set.h index 493de34a20..660203ead7 100644 --- a/yt/yt/library/profiling/solomon/sensor_set.h +++ b/yt/yt/library/profiling/solomon/sensor_set.h @@ -190,7 +190,7 @@ public: bool IsEmpty() const; - void Profile(const TProfiler& profiler); + void Profile(const TWeakProfiler& profiler); void ValidateOptions(const TSensorOptions& options); void AddCounter(TCounterStatePtr counter); diff --git a/yt/yt/library/profiling/unittests/exporter_ut.cpp b/yt/yt/library/profiling/unittests/exporter_ut.cpp index 2187b43b7d..0b4cbbbbd8 100644 --- a/yt/yt/library/profiling/unittests/exporter_ut.cpp +++ b/yt/yt/library/profiling/unittests/exporter_ut.cpp @@ -42,6 +42,19 @@ TEST(TSolomonExporter, MemoryLeak) exporter->Stop(); } +TEST(TSolomonExporter, MemoryLeakWithSelfProfiling) +{ + auto registry = New<TSolomonRegistry>(); + auto counter = TProfiler{registry, "yt"}.Counter("/foo"); + + auto config = New<TSolomonExporterConfig>(); + config->GridStep = TDuration::Seconds(1); + config->EnableCoreProfilingCompatibility = true; + config->EnableSelfProfiling = true; + + auto exporter = New<TSolomonExporter>(config, registry); +} + TEST(TSolomonExporter, ReadJsonHistogram) { auto registry = New<TSolomonRegistry>(); |