wallace created this revision. wallace added reviewers: clayborg, jingham. Herald added subscribers: dang, arphaman, mgorny. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
D45547 <https://reviews.llvm.org/D45547> added a while ago the skeleton for some target stats, along with a command to enable, disable, and dump them. However, it seems it wasn't further developed. My team is in need of a bunch of other target stats so that we can fix some bottlenecks. Some of these stats are related to expressions (e.g. # of expr eval that trigger global lookups, time per expr eval), and some others are are related to modules (e.g. amount of debug info parsed, time spent indexing dwarf, etc.). In order to collect this metrics, I'm proposing improving the existing code and create a TargetStats class, that can keep track of them. Some things to consider: - I think it's useful to have colletion enabled by default. You might encounter some perf issues and you might want to dump the stats right away, instead of rerunning the debug session but this time with collection enabled. - The information that is very cheap to collect, should be collected on the fly, e.g. # of failed frame vars. - The information that is expensive to collect, should be collected at retrieval time (e.g. when invoking ToJSON or Dump). That way the collection can be enabled by default without paying any noticeable penalty. As an interesting case, I added a metric for the amount of time spent indexing dwarf per module, which is not as trivial as the other existing metrics because the Target is not available there. However, it was easy to implement and can be extended to all symbol files. This metric is collected at retrieval time. This doesn't account for cases in which a dynamic library is unloaded at runtime, but I'm just making the current changes just for demostration purposes. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D107407 Files: lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Target/Target.h lldb/include/lldb/Target/TargetStats.h lldb/include/lldb/lldb-forward.h lldb/source/API/SBTarget.cpp lldb/source/Commands/CommandObjectExpression.cpp lldb/source/Commands/CommandObjectFrame.cpp lldb/source/Commands/CommandObjectStats.cpp lldb/source/Commands/Options.td lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Target/CMakeLists.txt lldb/source/Target/Target.cpp lldb/source/Target/TargetStats.cpp
Index: lldb/source/Target/TargetStats.cpp =================================================================== --- /dev/null +++ lldb/source/Target/TargetStats.cpp @@ -0,0 +1,101 @@ +//===-- Target.cpp --------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Target/TargetStats.h" + +#include "lldb/Core/Module.h" +#include "lldb/Symbol/SymbolFile.h" +#include "lldb/Target/Target.h" + +using namespace lldb; +using namespace lldb_private; +using namespace llvm; + +void TargetStats::Operation::Notify(bool success) { + if (success) + successes++; + else + failures++; +} + +void TargetStats::NotifyExprEval(bool success) { m_expr_eval.Notify(success); } + +void TargetStats::NotifyFrameVar(bool success) { m_frame_var.Notify(success); } + +void TargetStats::CollectModuleStats(Target &target) { + for (ModuleSP module_sp : target.GetImages().Modules()) { + m_module_stats[module_sp->GetFileSpec().GetPath()] = { + module_sp->GetSymbolFile()->GetSymbolLoadingTime()}; + } +} + +void TargetStats::Dump(Stream &s, Target &target) { + CollectModuleStats(target); + s << llvm::formatv("Number of expr evaluation successes: {0}\n", + m_expr_eval.successes) + .str(); + s << llvm::formatv("Number of expr evaluation failures: {0}\n", + m_expr_eval.failures) + .str(); + s << llvm::formatv("Number of frame var successes: {0}\n", + m_frame_var.successes) + .str(); + s << llvm::formatv("Number of frame var failures: {0}\n", + m_frame_var.failures) + .str(); + + s << "Modules: \n"; + + std::for_each(m_module_stats.begin(), m_module_stats.end(), + [&](const std::pair<std::string, ModuleStats> &module_entry) { + s << " " << module_entry.first << ":\n"; + s << " Symbol loading time in seconds: "; + if (module_entry.second.symbol_loading_time) + s << llvm::formatv( + "{0:f9}", + module_entry.second.symbol_loading_time->count()) + .str(); + else + s << "not loaded"; + s << "\n"; + }); +} + +json::Value TargetStats::Operation::ToJSON() const { + return json::Value(json::Object{ + {"successes", successes}, + {"failures", failures}, + }); +} + +json::Value TargetStats::ModuleStats::ToJSON() const { + if (symbol_loading_time) { + return json::Value( + json::Object{{"symbolLoadingTimeInSec:", + formatv("{0:f9}", symbol_loading_time->count()).str()}}); + } else { + return json::Value(json::Object{{"symbolLoadingTimeInSec:", nullptr}}); + } +} + +json::Value TargetStats::ToJSON(Target &target) { + CollectModuleStats(target); + + json::Object modules; + std::for_each( + m_module_stats.begin(), m_module_stats.end(), + [&](const std::pair<std::string, ModuleStats> &module_entry) { + modules.insert({module_entry.first, module_entry.second.ToJSON()}); + }); + + return json::Value(json::Object{ + {"exprEval", m_expr_eval.ToJSON()}, + {"frameVal", m_frame_var.ToJSON()}, + {"modules", std::move(modules)}, + }); +} Index: lldb/source/Target/Target.cpp =================================================================== --- lldb/source/Target/Target.cpp +++ lldb/source/Target/Target.cpp @@ -99,8 +99,7 @@ m_valid(true), m_suppress_stop_hooks(false), m_is_dummy_target(is_dummy_target), m_frame_recognizer_manager_up( - std::make_unique<StackFrameRecognizerManager>()), - m_stats_storage(static_cast<int>(StatisticKind::StatisticMax)) + std::make_unique<StackFrameRecognizerManager>()) { SetEventName(eBroadcastBitBreakpointChanged, "breakpoint-changed"); Index: lldb/source/Target/CMakeLists.txt =================================================================== --- lldb/source/Target/CMakeLists.txt +++ lldb/source/Target/CMakeLists.txt @@ -43,6 +43,7 @@ SystemRuntime.cpp Target.cpp TargetList.cpp + TargetStats.cpp Thread.cpp ThreadCollection.cpp ThreadList.cpp Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -320,6 +320,8 @@ /// Same as GetLanguage() but reports all C++ versions as C++ (no version). static lldb::LanguageType GetLanguageFamily(DWARFUnit &unit); + llvm::Optional<std::chrono::duration<double>> GetSymbolLoadingTime() override; + protected: typedef llvm::DenseMap<const DWARFDebugInfoEntry *, lldb_private::Type *> DIEToTypePtr; Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -514,6 +514,13 @@ std::make_unique<ManualDWARFIndex>(*GetObjectFile()->GetModule(), *this); } +llvm::Optional<std::chrono::duration<double>> +SymbolFileDWARF::GetSymbolLoadingTime() { + if (!m_index) + return llvm::None; + return m_index->GetSymbolLoadingTime(); +} + bool SymbolFileDWARF::SupportedVersion(uint16_t version) { return version >= 2 && version <= 5; } Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h +++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h @@ -55,6 +55,8 @@ void Dump(Stream &s) override; + llvm::Optional<std::chrono::duration<double>> GetSymbolLoadingTime() override; + private: struct IndexSet { NameToDIE function_basenames; @@ -67,6 +69,7 @@ NameToDIE namespaces; }; void Index(); + void DoIndex(); void IndexUnit(DWARFUnit &unit, SymbolFileDWARFDwo *dwp, IndexSet &set); static void IndexUnitImpl(DWARFUnit &unit, @@ -80,6 +83,7 @@ llvm::DenseSet<dw_offset_t> m_units_to_avoid; IndexSet m_set; + llvm::Optional<std::chrono::duration<double>> m_loading_time; }; } // namespace lldb_private Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp @@ -24,6 +24,18 @@ using namespace lldb; void ManualDWARFIndex::Index() { + std::chrono::duration<double> t1 = + std::chrono::steady_clock::now().time_since_epoch(); + DoIndex(); + m_loading_time = std::chrono::steady_clock::now().time_since_epoch() - t1; +} + +llvm::Optional<std::chrono::duration<double>> +ManualDWARFIndex::GetSymbolLoadingTime() { + return m_loading_time; +} + +void ManualDWARFIndex::DoIndex() { if (!m_dwarf) return; Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h @@ -61,6 +61,10 @@ virtual void Dump(Stream &s) = 0; + virtual llvm::Optional<std::chrono::duration<double>> GetSymbolLoadingTime() { + return llvm::None; + } + protected: Module &m_module; Index: lldb/source/Commands/Options.td =================================================================== --- lldb/source/Commands/Options.td +++ lldb/source/Commands/Options.td @@ -1272,3 +1272,8 @@ def trace_schema_verbose : Option<"verbose", "v">, Group<1>, Desc<"Show verbose trace schema logging for debugging the plug-in.">; } + +let Command = "statistics dump" in { + def statistics_dump_json: Option<"json", "j">, Group<1>, + Desc<"Dump the statistics in JSON format.">; +} Index: lldb/source/Commands/CommandObjectStats.cpp =================================================================== --- lldb/source/Commands/CommandObjectStats.cpp +++ lldb/source/Commands/CommandObjectStats.cpp @@ -7,95 +7,82 @@ //===----------------------------------------------------------------------===// #include "CommandObjectStats.h" + +#include "lldb/Host/OptionParser.h" #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Target/Target.h" using namespace lldb; using namespace lldb_private; -class CommandObjectStatsEnable : public CommandObjectParsed { -public: - CommandObjectStatsEnable(CommandInterpreter &interpreter) - : CommandObjectParsed(interpreter, "enable", - "Enable statistics collection", nullptr, - eCommandProcessMustBePaused) {} - - ~CommandObjectStatsEnable() override = default; - -protected: - bool DoExecute(Args &command, CommandReturnObject &result) override { - Target &target = GetSelectedOrDummyTarget(); +#pragma mark CommandObjectStats - if (target.GetCollectingStats()) { - result.AppendError("statistics already enabled"); - return false; - } +#define LLDB_OPTIONS_statistics_dump +#include "CommandOptions.inc" - target.SetCollectingStats(true); - result.SetStatus(eReturnStatusSuccessFinishResult); - return true; - } -}; - -class CommandObjectStatsDisable : public CommandObjectParsed { +class CommandObjectStatsDump : public CommandObjectParsed { public: - CommandObjectStatsDisable(CommandInterpreter &interpreter) - : CommandObjectParsed(interpreter, "disable", - "Disable statistics collection", nullptr, - eCommandProcessMustBePaused) {} - - ~CommandObjectStatsDisable() override = default; + class CommandOptions : public Options { + public: + CommandOptions() : Options() { OptionParsingStarting(nullptr); } + + Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg, + ExecutionContext *execution_context) override { + Status error; + const int short_option = m_getopt_table[option_idx].val; + + switch (short_option) { + case 'j': { + m_json = true; + break; + } + default: + llvm_unreachable("Unimplemented option"); + } + return error; + } -protected: - bool DoExecute(Args &command, CommandReturnObject &result) override { - Target &target = GetSelectedOrDummyTarget(); + void OptionParsingStarting(ExecutionContext *execution_context) override { + m_json = false; + } - if (!target.GetCollectingStats()) { - result.AppendError("need to enable statistics before disabling them"); - return false; + llvm::ArrayRef<OptionDefinition> GetDefinitions() override { + return llvm::makeArrayRef(g_statistics_dump_options); } - target.SetCollectingStats(false); - result.SetStatus(eReturnStatusSuccessFinishResult); - return true; - } -}; + bool m_json; + }; -class CommandObjectStatsDump : public CommandObjectParsed { -public: CommandObjectStatsDump(CommandInterpreter &interpreter) : CommandObjectParsed(interpreter, "dump", "Dump statistics results", - nullptr, eCommandProcessMustBePaused) {} + nullptr, eCommandProcessMustBePaused), + m_options() {} ~CommandObjectStatsDump() override = default; + Options *GetOptions() override { return &m_options; } + protected: bool DoExecute(Args &command, CommandReturnObject &result) override { - Target &target = GetSelectedOrDummyTarget(); - - uint32_t i = 0; - for (auto &stat : target.GetStatistics()) { - result.AppendMessageWithFormat( - "%s : %u\n", - lldb_private::GetStatDescription( - static_cast<lldb_private::StatisticKind>(i)) - .c_str(), - stat); - i += 1; - } + Target &target = m_exe_ctx.GetTargetRef(); + TargetStats &stats = target.GetStatistics(); + + if (m_options.m_json) + result.AppendMessageWithFormatv("{0:2}", stats.ToJSON(target)); + else + stats.Dump(result.GetOutputStream(), target); + result.SetStatus(eReturnStatusSuccessFinishResult); return true; } + + CommandOptions m_options; }; CommandObjectStats::CommandObjectStats(CommandInterpreter &interpreter) : CommandObjectMultiword(interpreter, "statistics", "Print statistics about a debugging session", "statistics <subcommand> [<subcommand-options>]") { - LoadSubCommand("enable", - CommandObjectSP(new CommandObjectStatsEnable(interpreter))); - LoadSubCommand("disable", - CommandObjectSP(new CommandObjectStatsDisable(interpreter))); LoadSubCommand("dump", CommandObjectSP(new CommandObjectStatsDump(interpreter))); } Index: lldb/source/Commands/CommandObjectFrame.cpp =================================================================== --- lldb/source/Commands/CommandObjectFrame.cpp +++ lldb/source/Commands/CommandObjectFrame.cpp @@ -707,11 +707,7 @@ // Increment statistics. bool res = result.Succeeded(); - Target &target = GetSelectedOrDummyTarget(); - if (res) - target.IncrementStats(StatisticKind::FrameVarSuccess); - else - target.IncrementStats(StatisticKind::FrameVarFailure); + GetSelectedOrDummyTarget().GetStatistics().NotifyFrameVar(res); return res; } Index: lldb/source/Commands/CommandObjectExpression.cpp =================================================================== --- lldb/source/Commands/CommandObjectExpression.cpp +++ lldb/source/Commands/CommandObjectExpression.cpp @@ -661,12 +661,12 @@ history.AppendString(fixed_command); } // Increment statistics to record this expression evaluation success. - target.IncrementStats(StatisticKind::ExpressionSuccessful); + target.GetStatistics().NotifyExprEval(true); return true; } // Increment statistics to record this expression evaluation failure. - target.IncrementStats(StatisticKind::ExpressionFailure); + target.GetStatistics().NotifyExprEval(false); result.SetStatus(eReturnStatusFailed); return false; } Index: lldb/source/API/SBTarget.cpp =================================================================== --- lldb/source/API/SBTarget.cpp +++ lldb/source/API/SBTarget.cpp @@ -218,35 +218,20 @@ if (!target_sp) return LLDB_RECORD_RESULT(data); - auto stats_up = std::make_unique<StructuredData::Dictionary>(); - int i = 0; - for (auto &Entry : target_sp->GetStatistics()) { - std::string Desc = lldb_private::GetStatDescription( - static_cast<lldb_private::StatisticKind>(i)); - stats_up->AddIntegerItem(Desc, Entry); - i += 1; - } + std::string json_str = + llvm::formatv("{0}", target_sp->GetStatistics().ToJSON(*target_sp)).str(); + data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str)); - data.m_impl_up->SetObjectSP(std::move(stats_up)); return LLDB_RECORD_RESULT(data); } void SBTarget::SetCollectingStats(bool v) { LLDB_RECORD_METHOD(void, SBTarget, SetCollectingStats, (bool), v); - - TargetSP target_sp(GetSP()); - if (!target_sp) - return; - return target_sp->SetCollectingStats(v); } bool SBTarget::GetCollectingStats() { LLDB_RECORD_METHOD_NO_ARGS(bool, SBTarget, GetCollectingStats); - - TargetSP target_sp(GetSP()); - if (!target_sp) - return false; - return target_sp->GetCollectingStats(); + return true; } SBProcess SBTarget::LoadCore(const char *core_file) { Index: lldb/include/lldb/lldb-forward.h =================================================================== --- lldb/include/lldb/lldb-forward.h +++ lldb/include/lldb/lldb-forward.h @@ -214,6 +214,7 @@ class Target; class TargetList; class TargetProperties; +class TargetStats; class Thread; class ThreadCollection; class ThreadList; Index: lldb/include/lldb/Target/TargetStats.h =================================================================== --- /dev/null +++ lldb/include/lldb/Target/TargetStats.h @@ -0,0 +1,59 @@ +//===-- TargetStats.h -------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_TARGET_TARGETSTATS_H +#define LLDB_TARGET_TARGETSTATS_H + +#include <chrono> +#include <vector> + +#include "llvm/Support/JSON.h" + +#include "lldb/Utility/Stream.h" +#include "lldb/lldb-forward.h" + +namespace lldb_private { + +class TargetStats { +public: + struct Operation { + void Notify(bool success); + + llvm::json::Value ToJSON() const; + + uint32_t successes = 0; + uint32_t failures = 0; + }; + + struct ModuleStats { + llvm::json::Value ToJSON() const; + + llvm::Optional<std::chrono::duration<double>> symbol_loading_time; + }; + + void NotifyExprEval(bool success); + + void NotifyFrameVar(bool success); + + void NotifyModuleSymbolParsing(std::chrono::duration<double> duration); + + llvm::json::Value ToJSON(Target &target); + + void Dump(Stream &s, Target &target); + +private: + void CollectModuleStats(Target &target); + + Operation m_expr_eval; + Operation m_frame_var; + std::map<std::string, ModuleStats> m_module_stats; +}; + +} // namespace lldb_private + +#endif // LLDB_TARGET_TARGETSTATS_H Index: lldb/include/lldb/Target/Target.h =================================================================== --- lldb/include/lldb/Target/Target.h +++ lldb/include/lldb/Target/Target.h @@ -28,6 +28,7 @@ #include "lldb/Target/ExecutionContextScope.h" #include "lldb/Target/PathMappingList.h" #include "lldb/Target/SectionLoadHistory.h" +#include "lldb/Target/TargetStats.h" #include "lldb/Target/ThreadSpec.h" #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/Broadcaster.h" @@ -1444,25 +1445,14 @@ static void ImageSearchPathsChanged(const PathMappingList &path_list, void *baton); - // Utilities for `statistics` command. + /// \{ + /// Utilities for `statistics` command. private: - std::vector<uint32_t> m_stats_storage; - bool m_collecting_stats = false; + TargetStats m_stats; public: - void SetCollectingStats(bool v) { m_collecting_stats = v; } - - bool GetCollectingStats() { return m_collecting_stats; } - - void IncrementStats(lldb_private::StatisticKind key) { - if (!GetCollectingStats()) - return; - lldbassert(key < lldb_private::StatisticKind::StatisticMax && - "invalid statistics!"); - m_stats_storage[key] += 1; - } - - std::vector<uint32_t> GetStatistics() { return m_stats_storage; } + TargetStats &GetStatistics() { return m_stats; } + /// \} private: /// Construct with optional file and arch. Index: lldb/include/lldb/Symbol/SymbolFile.h =================================================================== --- lldb/include/lldb/Symbol/SymbolFile.h +++ lldb/include/lldb/Symbol/SymbolFile.h @@ -299,6 +299,10 @@ virtual void Dump(Stream &s); + virtual llvm::Optional<std::chrono::duration<double>> GetSymbolLoadingTime() { + return llvm::None; + } + protected: void AssertModuleLock(); virtual uint32_t CalculateNumCompileUnits() = 0;
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
