| Message ID | Zml6JmH5cbS7-HfZ@uudg.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | rtla/osnoise: Better report when histogram is empty | expand |
On Wed, 12 Jun 2024, Luis Claudio R. Goncalves wrote: > When osnoise hist does not observe any samples above the threshold, > no entries are recorded and the final report shows empty entries > for the usual statistics (count, min, max, avg): > > [~]# osnoise hist -d 5s -T 500 > # RTLA osnoise histogram > # Time unit is microseconds (us) > # Duration: 0 00:00:05 > Index > over: > count: > min: > avg: > max: > > That could lead users to confusing interpretations of the results. > > A simple solution is to report 0 for count and the statistics, making it > clear that no noise (above the defined threshold) was observed: > > [~]# osnoise hist -d 5s -T 500 > # RTLA osnoise histogram > # Time unit is microseconds (us) > # Duration: 0 00:00:05 > Index > over: 0 > count: 0 > min: 0 > avg: 0 > max: 0 > > > Signed-off-by: Luis Claudio R. Goncalves <lgoncalv@redhat.com> > --- > tools/tracing/rtla/src/osnoise_hist.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c > index 7be17d09f7e85..214e2c93fde01 100644 > --- a/tools/tracing/rtla/src/osnoise_hist.c > +++ b/tools/tracing/rtla/src/osnoise_hist.c > @@ -374,6 +374,7 @@ osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *too > { > struct osnoise_hist_data *data = tool->data; > struct trace_instance *trace = &tool->trace; > + int has_samples = 0; > int bucket, cpu; > int total; > > @@ -402,11 +403,25 @@ osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *too > continue; > } > > + /* There are samples above the threshold */ IMHO The comment isn't needed because the variable had_samples is descriptive, but it's not a big deal either > + has_samples = 1; > trace_seq_printf(trace->seq, "\n"); > trace_seq_do_printf(trace->seq); > trace_seq_reset(trace->seq); > } > > + /* > + * If no samples were recorded, skip calculations, print zeroed statistics > + * and return. > + */ > + if (!has_samples) { > + trace_seq_reset(trace->seq); > + trace_seq_printf(trace->seq, "over: 0\ncount: 0\nmin: 0\navg: 0\nmax: 0\n"); > + trace_seq_do_printf(trace->seq); > + trace_seq_reset(trace->seq); > + return; > + } > + > if (!params->no_index) > trace_seq_printf(trace->seq, "over: "); > > > > Reviewed-by: John Kacur <jkacur@redhat.com>
On 6/12/24 16:12, John Kacur wrote: >> + /* There are samples above the threshold */ > IMHO The comment isn't needed because the variable had_samples is > descriptive, but it's not a big deal either Yeah, I would not do it too, but it is fine. I am adding it to my queue. Thanks Luis and John! -- Daniel
diff --git a/tools/tracing/rtla/src/osnoise_hist.c b/tools/tracing/rtla/src/osnoise_hist.c index 7be17d09f7e85..214e2c93fde01 100644 --- a/tools/tracing/rtla/src/osnoise_hist.c +++ b/tools/tracing/rtla/src/osnoise_hist.c @@ -374,6 +374,7 @@ osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *too { struct osnoise_hist_data *data = tool->data; struct trace_instance *trace = &tool->trace; + int has_samples = 0; int bucket, cpu; int total; @@ -402,11 +403,25 @@ osnoise_print_stats(struct osnoise_hist_params *params, struct osnoise_tool *too continue; } + /* There are samples above the threshold */ + has_samples = 1; trace_seq_printf(trace->seq, "\n"); trace_seq_do_printf(trace->seq); trace_seq_reset(trace->seq); } + /* + * If no samples were recorded, skip calculations, print zeroed statistics + * and return. + */ + if (!has_samples) { + trace_seq_reset(trace->seq); + trace_seq_printf(trace->seq, "over: 0\ncount: 0\nmin: 0\navg: 0\nmax: 0\n"); + trace_seq_do_printf(trace->seq); + trace_seq_reset(trace->seq); + return; + } + if (!params->no_index) trace_seq_printf(trace->seq, "over: ");
When osnoise hist does not observe any samples above the threshold, no entries are recorded and the final report shows empty entries for the usual statistics (count, min, max, avg): [~]# osnoise hist -d 5s -T 500 # RTLA osnoise histogram # Time unit is microseconds (us) # Duration: 0 00:00:05 Index over: count: min: avg: max: That could lead users to confusing interpretations of the results. A simple solution is to report 0 for count and the statistics, making it clear that no noise (above the defined threshold) was observed: [~]# osnoise hist -d 5s -T 500 # RTLA osnoise histogram # Time unit is microseconds (us) # Duration: 0 00:00:05 Index over: 0 count: 0 min: 0 avg: 0 max: 0 Signed-off-by: Luis Claudio R. Goncalves <lgoncalv@redhat.com> --- tools/tracing/rtla/src/osnoise_hist.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)