[Lldb-commits] [PATCH] D112279: Add modules stats into the "statistics dump" command.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/include/lldb/Target/Statistics.h:106
   StatsSuccessFail m_frame_var{"frameVariable"};
+  std::vector<ModuleStats> m_modules;
+
----------------
JDevlieghere wrote:
> As modules can span (and survive) targets, I'm confused this is part of the target stats? 
In order to create performance tests that can track and identify regressions, and because modules have no backlink to the lldb_private::Target object that is eventually might get added to, we can't get any signal in how long a target takes to load up and parse the modules (and soon debug info) that the target contains without asking each module to report the time it took the parse/index itself. 

The stats of how long it took to load the module in the first place is very important information for tracking regressions. Not sure if there is a better way to do this. 

We could iterate over all of the modules from the global module list, but I do like the idea of adding up the contributions for a given target by iterating over its current modules. But if you have more than one debug session going, I would hate to report all of the modules from both targets, possibly including all of the .o files from DWARF in .o files and then try to make any sense of the information. 

One idea would be to emit stats on all modules in a top level list like "modules", which would contain all of the stats for all modules loaded into LLDB in the global module list, and then have a "targetModules" which could just be an array of UUID values that refer to the modules in the global "modules" list of dicts? But this still doesn't change the fact that aggregating the values at the target level would be off.

I am open to ideas on how to report this better. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112279