[Lldb-commits] [lldb] [LLDB][Data Formatters] Calculate average and total time for summary providers within lldb (PR #102708)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 16 13:07:19 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) {
----------------
Jlalond wrote:
@Michael137 I think both the proposal you're approaching and the shared ptr flow can work. I think the primary contention will be whose job atomicity will be. Considering we're building on top of the established pattern within Elapsed time, I prefer SummaryStatistics to be itself thread safe.
As for the cache, I think my primary concern was the double lookup on both the `get` and the `update` calls, with our current flow only ever checking twice during creation of a new statistics object. I think this is the easiest flow because now we only have to think about one lock at runtime, the lock to get or insert into the summary cache (and the lock to render it as a json).
I am by far the least experienced person here, so I really only espouse my say as the author, but I think for minimum footguns around lifetimes on multiple threads, `SummaryStatisticsSP` is the way to go.
https://github.com/llvm/llvm-project/pull/102708
More information about the lldb-commits
mailing list