[Lldb-commits] [PATCH] D137191: [lldb] Add information on type systems to statistics dump command

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/Target/Statistics.cpp:258
     debug_index_time += module_stat.debug_index_time;
     debug_info_size += module_stat.debug_info_size;
+    json::Value module_stat_json = module_stat.ToJSON();
----------------
Seems like we should be populating module_stat to contain a dictionary of TypeSystem plug-in name to stats. So adding something to the ModuleStats structure like:

```
std::map<std::string, json::Value> type_system_stats;
```
Then the loop below would look like:
```
module->ForEachTypeSystem([&](TypeSystem *ts) {
  if (auto stats = ts->ReportStatistics())
    module_stat. type_system_stats[ts->GetPluginName()] = stats.value();
  return true;
});
```
We currently don't have each type system reporting a plug-in name, but that would be easy to add to the two TypeSystem plug-ins. ModuleStat::ToJSON() would need to be modified to emit a "typeSystemInfo" only if there are any stats in the "module_stat.type_system_stats" member.


================
Comment at: lldb/source/Target/Statistics.cpp:266
+        auto stats = ts->ReportStatistics();
+        if (stats.hasValue()) {
+          module_stat_obj->try_emplace("TypeSystemInfo", stats.getValue());
----------------
Remove {} for single line if statement per llvm coding guidelines


================
Comment at: lldb/source/Target/Statistics.cpp:267
+        if (stats.hasValue()) {
+          module_stat_obj->try_emplace("TypeSystemInfo", stats.getValue());
+        }
----------------
We have been doing camel case, but starting with lower case. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137191/new/

https://reviews.llvm.org/D137191