Re: [PATCH] perf test stat_bpf_counter.sh: Remove comparison of separate runs

[Namhyung Kim <namhyung@kernel.org>]

On Thu, Jun 06, 2024 at 03:09:43PM +0200, Veronika Molnarova wrote:
> 
> 
> On 6/5/24 02:31, Namhyung Kim wrote:
> > On Tue, Jun 04, 2024 at 05:31:11PM +0200, vmolnaro@redhat.com wrote:
> >> From: Veronika Molnarova <vmolnaro@redhat.com>
> >>
> >> The test has been failing for some time when two separate runs of
> >> perf benchmarks are recorded and the counts of the samples are compared,
> >> while once the recording was done with option --bpf-counters and once
> >> without it. It is expected that the count of the samples should within
> >> a certain range, firstly the difference should have been within 10%,
> >> which was then later raised to 20%. However, the test case keeps failing
> >> on certain architectures as recording the same benchmark can provide
> >> completely different counts samples based on the current load of the
> >> system.
> >>
> >> Sampling two separate runs on intel-eaglestream-spr-13 of "perf stat
> >> --no-big-num -e cycles -- perf bench sched messaging -g 1 -l 100 -t":
> >>
> >>  Performance counter stats for 'perf bench sched messaging -g 1 -l 100 -t':
> >>
> >>          396782898      cycles
> >>
> >>        0.010051983 seconds time elapsed
> >>
> >>        0.008664000 seconds user
> >>        0.097058000 seconds sys
> >>
> >>  Performance counter stats for 'perf bench sched messaging -g 1 -l 100 -t':
> >>
> >>         1431133032      cycles
> >>
> >>        0.021803714 seconds time elapsed
> >>
> >>        0.023377000 seconds user
> >>        0.349918000 seconds sys
> >>
> >> , which is ranging from 400mil to 1400mil samples.
> >>
> >> From the testing point of view, it does not make sense to compare two
> >> separate runs against each other when the conditions may change
> >> significantly. Remove the comparison of two separate runs and check only
> >> whether the stating works as expected for the --bpf-counters option. Compare
> >> the samples count only when the samples are recorded simultaneously
> >> ensuring the same conditions.
> > 
> > Hmm.. but having a test which checks if the output is sane can be
> > useful.  If it's a problem of dynamic changes in cpu cycles, maybe
> > we can use 'instructions' event instead (probably with :u) to get
> > more stable values?
> > 
> > Thanks,
> > Namhyung
> > 
> 
> Well but isn't it better to check the sanity of the output if the counters are recorded
> for a load with the same conditions as has been added in patch d9bd1d4
> "perf test bpf-counters: Add test for BPF event modifier" utilizing the event modifiers: 
> perf stat --no-big-num -e cycles/name=base_cycles/,cycles/name=bpf_cycles/b -- $workload?
> Comparing two separate runs is more based on the stability of the workload than the
> bpf-counters option, which then causes the issue of defining the threshold of what values
> are still within the sane range.

Right, it'd be better if we can run once and the compare the counters.
But I don't think if --bpf-counters works only for a specific event.

> 
> But you are right that another possible fix for this issue is to change the workload or the
> event to get some more stable values, which could be comparable.

Yep, let's try with 'instructions:u'.

Thanks,
Namhyung