[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-21 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Fixed buildbots with: commit 910838f07da7872d2b7cca5b07d64ea9915b6767 (HEAD -> main, origin/main, origin/HEAD) Author: Greg Clayton Date: Thu Oct 21 14:21:36 2021 -0700 Fix buildbots after

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-21 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D111686#3079352 , @teemperor wrote: > In D111686#3079248 , @clayborg > wrote: > >> So there is a buildbot failure due to an expression which succeeds, but >> because the

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-21 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment. In D111686#3079248 , @clayborg wrote: > So there is a buildbot failure due to an expression which succeeds, but > because the "expression" command options don't apply cleanly to the > expression result the "expression"

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-21 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. So there is a buildbot failure due to an expression which succeeds, but because the "expression" command options don't apply cleanly to the expression result the "expression" command fails. Question: do we want "expressionEvaluation" to truly track expression

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-21 Thread Greg Clayton via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd7b338537cf3: Modify statistics dump to dump JSON. (authored by clayborg). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111686/new/

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-21 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. I'm not super happy with the debugger stats as they are right here. Given that it only affects a subset of this patch and I don't want to block other work that depends on this,

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg updated this revision to Diff 381106. clayborg added a comment. Rename GlobalStats to DebuggerStats. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111686/new/ https://reviews.llvm.org/D111686 Files: lldb/include/lldb/Target/Process.h

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-20 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/include/lldb/Target/Statistics.h:96 +}; + +class GlobalStats { JDevlieghere wrote: > Do we expect there to be something like `DebuggerStats`? I think it would be > nice from a hierarchy perspective that Global

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-20 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: lldb/include/lldb/Target/Statistics.h:96 +}; + +class GlobalStats { Do we expect there to be something like `DebuggerStats`? I think it would be nice from a hierarchy perspective that Global Stats have a map of

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-18 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Friendly ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111686/new/ https://reviews.llvm.org/D111686 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Right now if you run: (lldb) statistics dump It will dump stats for the current target only as one target dictionary. We can change this if desired. (lldb) statistics dump --all-targets will dump an dictionary that contains a top level "targets" key/value pair where

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Ok, take a look now. I added a GlobalStats class that contains the "enable/disable" state of the statistics gathering. This shows how things can be organized and dumped. A few things about the current approach: Stats classes in Statistics.h are designed to be able to

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg updated this revision to Diff 379860. clayborg added a comment. Herald added a subscriber: dang. Fixed issues identified in first review round: - typos in comments - create GlobalStats class to contain the global statistics - Make sure all code is using the new StatsClock,

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-14 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: lldb/include/lldb/Target/Statistics.h:23 +struct SuccessFailStats { + void Notify(bool success); + wallace wrote: > JDevlieghere wrote: > > I'd use an enum class with `Success` and `Failure` here instead of a bool >

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-13 Thread walter erquinigo via Phabricator via lldb-commits
wallace added inline comments. Comment at: lldb/include/lldb/Target/Process.h:241 - void BumpStopID() { -m_stop_id++; + uint32_t BumpStopID() { +const uint32_t prev_stop_id = m_stop_id++; add a comment saying that this returns the stop id with the

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-13 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. I have a few comments but I think this is going in a direction that will work for both of us. Once this lands I can sign up for the work to untie the statistics form the target. Comment at: lldb/include/lldb/Target/Statistics.h:23 +struct

[Lldb-commits] [PATCH] D111686: Modify "statistics dump" to dump JSON.

2021-10-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg created this revision. clayborg added reviewers: jingham, labath, aprantl, JDevlieghere, aadsm, wallace, jdoerfert. Herald added a subscriber: mgorny. clayborg requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. This patch is a