[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg abandoned this revision. clayborg added a comment. This was submitted as a series of patches that updated "statistics dump" to emit JSON: https://reviews.llvm.org/D111686 https://reviews.llvm.org/D112279 https://reviews.llvm.org/D112501 https://reviews.llvm.org/D112683 https://reviews.l

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

wallace added a comment. is this going to be abandoned? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110804/new/ https://reviews.llvm.org/D110804 ___ lldb-commits mailing list lldb-commits@lists.llvm.or

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. I have broken out the target only settings to make this patch smaller into: https://reviews.llvm.org/D111686 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110804/new/ https://reviews.llvm.org/D110804

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. In D110804#3037504 , @JDevlieghere wrote: > Thanks Greg! Following Pavel's suggestion in D110893 > , could you split up this patch into > smaller ones? I think that will make it a lot easier t

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

JDevlieghere added a comment. Thanks Greg! Following Pavel's suggestion in D110893 , could you split up this patch into smaller ones? I think that will make it a lot easier to review and discuss. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. In D110804#3035276 , @JDevlieghere wrote: > In D110804#3033689 , @JDevlieghere > wrote: > >> We have (short term) plans to start using the statistics to collect more >> data. Vedant cr

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg updated this revision to Diff 376421. clayborg added a comment. Changes: - Removed "target metrics" and took over "statistics dump" - Hooked old statistics into for expression evaluation and frame variable success and fails into this patch - No more textual output for "statistics dump",

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

JDevlieghere added a comment. In D110804#3033689 , @JDevlieghere wrote: > We have (short term) plans to start using the statistics to collect more > data. Vedant created some patches last year that I've rebased but haven't > landed yet. I'll try to get

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

jingham added a comment. In D110804#3035052 , @clayborg wrote: > Ok, so how does this sound: > > - This new command will replace the existing "statistics dump" command and > emit JSON only by default > - The "statistics enable/disable" will stay in place

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. Ok, so how does this sound: - This new command will replace the existing "statistics dump" command and emit JSON only by default - The "statistics enable/disable" will stay in place and allow expensive metric gathering to be enabled/disabled if needed. If the gathering

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

jingham added a comment. In D110804#3034878 , @clayborg wrote: > I would like to get a consensus on if we should move this functionality to > "statistics" or not. The reasons that I didn't do it were: > > - "statistics" as a top level method doesn't real

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

JDevlieghere added a comment. In D110804#3034878 , @clayborg wrote: > I would like to get a consensus on if we should move this functionality to > "statistics" or not. The reasons that I didn't do it were: > > - "statistics" as a top level method doesn't

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. I would like to get a consensus on if we should move this functionality to "statistics" or not. The reasons that I didn't do it were: - "statistics" as a top level method doesn't really do what I wanted here, which was "target statistics", or stats that are related onl

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg marked 6 inline comments as done. clayborg added inline comments. Comment at: lldb/include/lldb/Core/Module.h:940 + /// ElapsedTime + double &GetSymtabParseTime() { return m_symtab_parse_time; } + double &GetSymtabNamesTime() { return m_symtab_names_time; } --

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg updated this revision to Diff 376358. clayborg added a comment. Changes: - Swithed from using "double" to using ElapsedTime::Duration - Stop using assertTrue in tests, and use better assertXXX methods Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg added a comment. In D110804#3032741 , @labath wrote: > I have a feeling that there's a lot of overlap between this and the > "statistics" command. As a user, I don't think I would be able to tell the > difference between these two things. > > H

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

JDevlieghere added a comment. In D110804#3032741 , @labath wrote: > I have a feeling that there's a lot of overlap between this and the > "statistics" command. As a user, I don't think I would be able to tell the > difference between these two things. >

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

teemperor added inline comments. Comment at: lldb/include/lldb/Core/Module.h:940 + /// ElapsedTime + double &GetSymtabParseTime() { return m_symtab_parse_time; } + double &GetSymtabNamesTime() { return m_symtab_names_time; } labath wrote: > teemperor wrote: >

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

labath added inline comments. Comment at: lldb/include/lldb/Core/Module.h:940 + /// ElapsedTime + double &GetSymtabParseTime() { return m_symtab_parse_time; } + double &GetSymtabNamesTime() { return m_symtab_names_time; } teemperor wrote: > Could the `double`

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

teemperor added subscribers: vsk, teemperor. teemperor added a comment. +Vedant who had/has some plans with the statistics command at some point IIRC. FWIW, I wanted to throw away the current statistics implementation for quite a while but I didn't have anything to replace it with. If we can rep

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

labath added a comment. I have a feeling that there's a lot of overlap between this and the "statistics" command. As a user, I don't think I would be able to tell the difference between these two things. Having two systems which do vaguely similar things does not seem like a good idea. Have yo

[Lldb-commits] [PATCH] D110804: Add a new command "target metrics".

clayborg created this revision. clayborg added reviewers: jingham, labath, aprantl, JDevlieghere, aadsm, wallace. Herald added subscribers: dang, pengfei, arphaman, mgorny, emaste. clayborg requested review of this revision. Herald added subscribers: lldb-commits, sstefan1, MaskRay. Herald added a