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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D110804#3032741 <https://reviews.llvm.org/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.
>
> Having two systems which do vaguely similar things does not seem like a good idea. Have you considered unifying them? Or maybe just deleting the old one, if its not used (I don't think it has gotten any improvements since it was initially introduced)?

I would be willing to take over the "statistics" command, but as it is written, it has to be enabled and disabled which I didn't like so I didn't want to have to worry about that. If we were to take over the "statistics" multiword command we would need to keep old "statistics enable" and "statistics disable" and make them no-ops and then add a JSON output. I started off this way, but it felt like a hack. And to be honest the statistics command doesn't do much. It also isn't placed correctly as I feel "target statistics" would be a better place and I would be fine renaming "metrics" to "statistics", but that would also be confusing to have two commands with the same name in different places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110804