[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 4 13:23:59 PDT 2024
================
@@ -174,6 +176,82 @@ struct StatisticsOptions {
std::optional<bool> m_include_transcript;
};
+/// A class that represents statistics about a TypeSummaryProviders invocations
+class SummaryStatistics {
+public:
+ explicit SummaryStatistics(std::string name, std::string impl_type)
+ : m_total_time(), m_impl_type(impl_type), m_name(name), m_count(0) {}
+
+ std::string GetName() const { return m_name; };
+ double GetTotalTime() const { return m_total_time.get().count(); }
+
+ uint64_t GetSummaryCount() const {
+ return m_count.load(std::memory_order_relaxed);
+ }
+
+ StatsDuration &GetDurationReference() { return m_total_time; };
+
+ std::string GetSummaryKindName() const { return m_impl_type; }
+
+ llvm::json::Value ToJSON() const;
+
+ /// Basic RAII class to increment the summary count when the call is complete.
+ class SummaryInvocation {
+ public:
+ SummaryInvocation(std::shared_ptr<SummaryStatistics> summary_stats)
+ : m_stats(summary_stats),
+ m_elapsed_time(summary_stats->GetDurationReference()) {}
+ ~SummaryInvocation() { m_stats->OnInvoked(); }
+
+ /// Delete the copy constructor and assignment operator to prevent
+ /// accidental double counting.
+ /// @{
+ SummaryInvocation(const SummaryInvocation &) = delete;
+ SummaryInvocation &operator=(const SummaryInvocation &) = delete;
+ /// @}
+
+ private:
+ std::shared_ptr<SummaryStatistics> m_stats;
+ ElapsedTime m_elapsed_time;
+ };
+
+private:
+ void OnInvoked() noexcept { m_count.fetch_add(1, std::memory_order_relaxed); }
+ lldb_private::StatsDuration m_total_time;
+ const std::string m_impl_type;
+ const std::string m_name;
+ std::atomic<uint64_t> m_count;
+};
+
+typedef std::shared_ptr<SummaryStatistics> SummaryStatisticsSP;
+
+/// A class that wraps a std::map of SummaryStatistics objects behind a mutex.
+class SummaryStatisticsCache {
+public:
+ /// Get the SummaryStatistics object for a given provider name, or insert
+ /// if statistics for that provider is not in the map.
+ SummaryStatisticsSP
+ GetSummaryStatisticsForProviderName(lldb_private::TypeSummaryImpl &provider) {
----------------
clayborg wrote:
So with your suggestion the RAII object would need to copy the string so that it could call `Update` at the and to find the stats and then update the time. This also means the RAII object needs to hold a reference to the `SummaryStatisticsCache` global object, which could get destructed as nothing is keeping it alive. So we need to have the RAII object copy the string and get a weak pointer to the SummaryStatisticsCache so it can safely update the results in the end.
This current solution hands out a `SummaryStatisticsSP` to the RAII object which will keep it alive so it can get updated, and if the `SummaryStatisticsCache` goes away, it is ok because we have a strong reference. Both solutions only do 1 lookup. `std::shared_ptr<T>` objects are thread safe.
The StringMap requires a value that can be copied when the map grows and re-allocation happen, so if we don't use a shared pointer, then we need `SummaryStatistics` to have a move operator.
So I don't mind this solution, but would like to hear other opinions
https://github.com/llvm/llvm-project/pull/102708
More information about the lldb-commits
mailing list