Messages in this thread | | | From | Ian Rogers <> | Date | Tue, 29 Nov 2022 21:01:11 -0800 | Subject | Re: [PATCH 14/15] perf stat: Rename "aggregate-number" to "cpu-count" in JSON |
| |
On Tue, Nov 29, 2022 at 2:46 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Sat, Nov 26, 2022 at 7:14 PM Ian Rogers <irogers@google.com> wrote: > > > > On Thu, Nov 24, 2022 at 11:51 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > Hi Ian, > > > > > > On Wed, Nov 23, 2022 at 3:31 PM Ian Rogers <irogers@google.com> wrote: > > > > > > > > On Wed, Nov 23, 2022 at 10:02 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > > > As the JSON output has been broken for a little while, I guess there are > > > > > not many users. Let's rename the field to more intuitive one. :) > > > > > > > > I'm not sure cpu-count is accurate. For example, an uncore counter in > > > > a dual socket machine may have a CPU mask of "0, 36", ie one event per > > > > socket. The aggregate-number in this case I believe is 2. > > > > > > You're right. In case of uncore events, it can be confusing. But in some > > > sense it could be thought as cpu count as well since it aggregates the > > > result from two cpus anyway. :) > > > > > > Note that the aggregate-number (or cpu-count) is only printed if users > > > requested one of aggregation options like --per-socket or --per-core. > > > In your example, then it could print 1 for each socket. > > > > > > But I think uncore events are different from core events, and hopefully > > > they have separate instances for different sockets or something already. > > > That means it doesn't need to use those aggregation options for them. > > > > > > Also the CSV output uses "cpus" for the same information. It'd be nice > > > we could have consistency. > > > > So in the original patch from Claire she'd passed the name "number" > > through to the json from the stat code. Having an integer called > > "number" isn't exactly intention revealing - thank you for your clean > > up work! :-) I switched "number" to be "aggregate number" as the > > number comes from the "data" aggregated and the code refers to it as > > aggregate data. I think aggregate-number is more consistent with the > > code, and cpu-count would look strange in the uncore case above where > > the number of CPUs (really hyperthreads) is 72. Perhaps we should also > > be outputting the aggregation mode with the number. Anyway, I think > > for the patch series I'd prefer we skipped this one and kept the rest. > > Right, I think we need a more general term to include non-cpu events. > But it seems Arnaldo already merged it. > > Arnaldo, do you want me to send a revert? > > Thanks, > Namhyung
This is also breaking the json output test:
$ perf test -vv 89 89: perf stat JSON output linter : --- start --- test child forked, pid 2116261 Checking json output: no args [Success] Checking json output: system wide [Success] Checking json output: interval [Success] Checking json output: event [Success] Checking json output: per thread [Success] Checking json output: per node Test failed for input: {"node" : "N0", "cpu-count" : 16, "counter-value" : "32.468431", "unit" : "msec", "event" : "cpu-clo ck", "event-runtime" : 32498339, "pcnt-running" : 100.00, "metric-value" : 19.450525, "metric-unit" : "CPUs utilized"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "52.000000", "unit" : "", "event" : "context-swi tches", "event-runtime" : 32471361, "pcnt-running" : 100.00, "metric-value" : 1.601556, "metric-unit " : "K/sec"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "16.000000", "unit" : "", "event" : "cpu-migrati ons", "event-runtime" : 32469950, "pcnt-running" : 100.00, "metric-value" : 492.786362, "metric-unit " : "/sec"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "57.000000", "unit" : "", "event" : "page-faults ", "event-runtime" : 32474408, "pcnt-running" : 100.00, "metric-value" : 1.755551, "metric-unit" : " K/sec"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "2958499.000000", "unit" : "", "event" : "cycles ", "event-runtime" : 32411643, "pcnt-running" : 100.00, "metric-value" : 0.091119, "metric-unit" : " GHz"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "3175893.000000", "unit" : "", "event" : "instru ctions", "event-runtime" : 32403573, "pcnt-running" : 100.00, "metric-value" : 1.073481, "metric-uni t" : "insn per cycle"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "688120.000000", "unit" : "", "event" : "branche s", "event-runtime" : 32391536, "pcnt-running" : 100.00, "metric-value" : 21.193509, "metric-unit" : "M/sec"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "17584.000000", "unit" : "", "event" : "branch-m isses", "event-runtime" : 32371972, "pcnt-running" : 100.00, "metric-value" : 2.555368, "metric-unit " : "of all branches"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "14377890.000000", "unit" : "", "event" : "slots ", "event-runtime" : 32350026, "pcnt-running" : 100.00, "metric-value" : 442.826757, "metric-unit" : "M/sec"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "3380921.000000", "unit" : "", "event" : "topdow n-retiring", "event-runtime" : 32350026, "pcnt-running" : 100.00, "metric-value" : 23.514767, "metri c-unit" : "Retiring"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "1444174.000000", "unit" : "", "event" : "topdow n-bad-spec", "event-runtime" : 32350026, "pcnt-running" : 100.00, "metric-value" : 10.044427, "metri c-unit" : "Bad Speculation"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "5899393.000000", "unit" : "", "event" : "topdow n-fe-bound", "event-runtime" : 32350026, "pcnt-running" : 100.00, "metric-value" : 41.031084, "metri c-unit" : "Frontend Bound"}
{"node" : "N0", "cpu-count" : 16, "counter-value" : "3653375.000000", "unit" : "", "event" : "topdow n-be-bound", "event-runtime" : 32350026, "pcnt-running" : 100.00, "metric-value" : 25.409722, "metri c-unit" : "Backend Bound"}
Traceback (most recent call last): File "/home/irogers/kernel.org/./tools/perf/tests/shell/lib/perf_json_output_lint.py", line 93, in <module> check_json_output(expected_items) File "/home/irogers/kernel.org/./tools/perf/tests/shell/lib/perf_json_output_lint.py", line 78, in check_json_output raise RuntimeError(f'Unexpected key: key={key} value={value}') RuntimeError: Unexpected key: key=cpu-count value=16 test child finished with -1 ---- end ---- perf stat JSON output linter: FAILED!
Thanks, Ian
| |