JDevlieghere created this revision.
JDevlieghere added reviewers: clayborg, labath, jingham.
Herald added a subscriber: yaxunl.
JDevlieghere requested review of this revision.
Move ownership of analytics out of Target and instead create a global Analytics
singleton. This is necessary to allow LLDB to record metrics in contexts where
a target either isn't available or doesn't make sense.
target).
https://reviews.llvm.org/D110895
Files:
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Utility/Analytics.h
lldb/source/API/SBTarget.cpp
lldb/source/API/SystemInitializerFull.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectStats.cpp
lldb/source/Utility/Analytics.cpp
Index: lldb/source/Utility/Analytics.cpp
===================================================================
--- lldb/source/Utility/Analytics.cpp
+++ lldb/source/Utility/Analytics.cpp
@@ -17,6 +17,9 @@
namespace lldb_private {
+static std::unique_ptr<Analytics> g_analytics;
+static std::mutex g_analytics_lock;
+
static unsigned GetNumberOfMetrics() {
return static_cast<unsigned>(Metric::MaxID);
}
@@ -67,4 +70,22 @@
return metrics_up;
}
+void Analytics::Initialize() { g_analytics = std::make_unique<Analytics>(); }
+
+void Analytics::Terminate() { g_analytics.reset(); }
+
+ExclusivelyLockedAnalytics::ExclusivelyLockedAnalytics() {
+ m_guard = std::unique_lock<std::mutex>(g_analytics_lock);
+}
+
+ExclusivelyLockedAnalytics::~ExclusivelyLockedAnalytics() {}
+
+Analytics *ExclusivelyLockedAnalytics::operator->() const {
+ return g_analytics.get();
+}
+
+ExclusivelyLockedAnalytics GetAnalytics() {
+ return ExclusivelyLockedAnalytics();
+}
+
} // namespace lldb_private
Index: lldb/source/Commands/CommandObjectStats.cpp
===================================================================
--- lldb/source/Commands/CommandObjectStats.cpp
+++ lldb/source/Commands/CommandObjectStats.cpp
@@ -24,14 +24,14 @@
protected:
bool DoExecute(Args &command, CommandReturnObject &result) override {
- Target &target = GetSelectedOrDummyTarget();
+ ExclusivelyLockedAnalytics analytics = GetAnalytics();
- if (target.GetCollectingStats()) {
+ if (analytics->IsEnabled()) {
result.AppendError("statistics already enabled");
return false;
}
- target.SetCollectingStats(true);
+ analytics->SetEnabled(true);
result.SetStatus(eReturnStatusSuccessFinishResult);
return true;
}
@@ -48,14 +48,14 @@
protected:
bool DoExecute(Args &command, CommandReturnObject &result) override {
- Target &target = GetSelectedOrDummyTarget();
+ ExclusivelyLockedAnalytics analytics = GetAnalytics();
- if (!target.GetCollectingStats()) {
+ if (!analytics->IsEnabled()) {
result.AppendError("need to enable statistics before disabling them");
return false;
}
- target.SetCollectingStats(false);
+ analytics->SetEnabled(false);
result.SetStatus(eReturnStatusSuccessFinishResult);
return true;
}
@@ -71,8 +71,7 @@
protected:
bool DoExecute(Args &command, CommandReturnObject &result) override {
- Target &target = GetSelectedOrDummyTarget();
- target.GetAnalytics().Print(result.GetOutputStream());
+ GetAnalytics()->Print(result.GetOutputStream());
result.SetStatus(eReturnStatusSuccessFinishResult);
return true;
}
Index: lldb/source/Commands/CommandObjectFrame.cpp
===================================================================
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -708,9 +708,8 @@
// Increment statistics.
bool res = result.Succeeded();
- Target &target = GetSelectedOrDummyTarget();
- Analytics &analytics = target.GetAnalytics();
- analytics.Record(res ? Metric::FrameVarSuccess : Metric::FrameVarFailure);
+ GetAnalytics()->Record(res ? Metric::FrameVarSuccess
+ : Metric::FrameVarFailure);
return res;
}
Index: lldb/source/Commands/CommandObjectExpression.cpp
===================================================================
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -642,7 +642,6 @@
}
Target &target = GetSelectedOrDummyTarget();
- Analytics &analytics = target.GetAnalytics();
if (EvaluateExpression(expr, result.GetOutputStream(),
result.GetErrorStream(), result)) {
@@ -660,11 +659,11 @@
fixed_command.append(m_fixed_expression);
history.AppendString(fixed_command);
}
- analytics.Record(Metric::ExpressionSuccess);
+ GetAnalytics()->Record(Metric::ExpressionSuccess);
return true;
}
- analytics.Record(Metric::ExpressionFailure);
+ GetAnalytics()->Record(Metric::ExpressionFailure);
result.SetStatus(eReturnStatusFailed);
return false;
}
Index: lldb/source/API/SystemInitializerFull.cpp
===================================================================
--- lldb/source/API/SystemInitializerFull.cpp
+++ lldb/source/API/SystemInitializerFull.cpp
@@ -76,6 +76,9 @@
#define LLDB_PLUGIN(p) LLDB_PLUGIN_INITIALIZE(p);
#include "Plugins/Plugins.def"
+ // Initialize analytics.
+ Analytics::Initialize();
+
// Initialize plug-ins in core LLDB
ProcessTrace::Initialize();
@@ -92,6 +95,9 @@
void SystemInitializerFull::Terminate() {
Debugger::SettingsTerminate();
+ // Terminate analytics.
+ Analytics::Terminate();
+
// Terminate plug-ins in core LLDB
ProcessTrace::Terminate();
Index: lldb/source/API/SBTarget.cpp
===================================================================
--- lldb/source/API/SBTarget.cpp
+++ lldb/source/API/SBTarget.cpp
@@ -57,6 +57,7 @@
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/Target.h"
#include "lldb/Target/TargetList.h"
+#include "lldb/Utility/Analytics.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/FileSpec.h"
@@ -218,7 +219,7 @@
if (!target_sp)
return LLDB_RECORD_RESULT(data);
- data.m_impl_up->SetObjectSP(target_sp->GetAnalytics().GetAsStructuredData());
+ data.m_impl_up->SetObjectSP(GetAnalytics()->GetAsStructuredData());
return LLDB_RECORD_RESULT(data);
}
Index: lldb/include/lldb/Utility/Analytics.h
===================================================================
--- lldb/include/lldb/Utility/Analytics.h
+++ lldb/include/lldb/Utility/Analytics.h
@@ -11,6 +11,7 @@
#include "lldb/Utility/StructuredData.h"
+#include <mutex>
#include <vector>
namespace lldb_private {
@@ -39,11 +40,36 @@
std::unique_ptr<StructuredData::Dictionary> GetAsStructuredData() const;
+ static void Initialize();
+ static void Terminate();
+
private:
std::vector<unsigned> m_counters;
bool m_collecting_analytics = false;
};
+/// RAII helper which acquires an exlusive lock on the global Analytics
+/// singleton on construction, and releases the lock on destruction.
+class ExclusivelyLockedAnalytics {
+public:
+ ExclusivelyLockedAnalytics();
+ ~ExclusivelyLockedAnalytics();
+
+ ExclusivelyLockedAnalytics(ExclusivelyLockedAnalytics &&) = default;
+
+ ExclusivelyLockedAnalytics &
+ operator=(ExclusivelyLockedAnalytics &&) = default;
+
+ Analytics *operator->() const;
+
+private:
+ std::unique_lock<std::mutex> m_guard;
+};
+
+/// Get a reference to the global Analytics singleton. This has UB when called
+/// without first calling \ref Analytics::Initialize.
+ExclusivelyLockedAnalytics GetAnalytics();
+
} // namespace lldb_private
#endif // LLDB_UTILITY_ANALYTICS_H
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -1450,16 +1450,9 @@
static void ImageSearchPathsChanged(const PathMappingList &path_list,
void *baton);
- // Utilities for `statistics` command.
-private:
- Analytics m_analytics;
-
public:
- void SetCollectingStats(bool v) { m_analytics.SetEnabled(v); }
-
- bool GetCollectingStats() { return m_analytics.IsEnabled(); }
-
- Analytics &GetAnalytics() { return m_analytics; }
+ void SetCollectingStats(bool v) { GetAnalytics()->SetEnabled(v); }
+ bool GetCollectingStats() { return GetAnalytics()->IsEnabled(); }
private:
/// Construct with optional file and arch.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
