Skip to content

Commit

Permalink
veristat: add ability to sort by stat's absolute value
Browse files Browse the repository at this point in the history
Add ability to sort results by absolute values of specified stats. This
is especially useful to find biggest deviations in comparison mode. When
comparing verifier change effect against a large base of BPF object
files, it's necessary to see big changes both in positive and negative
directions, as both might be a signal for regressions or bugs.

The syntax is natural, e.g., adding `-s '|insns_diff|'^` will instruct
veristat to sort by absolute value of instructions difference in
ascending order.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20231108051430.1830950-1-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Andrii Nakryiko authored and Alexei Starovoitov committed Nov 10, 2023
1 parent 7f7c436 commit 5d4a7aa
Showing 1 changed file with 56 additions and 12 deletions.
68 changes: 56 additions & 12 deletions tools/testing/selftests/bpf/veristat.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <libelf.h>
#include <gelf.h>
#include <float.h>
#include <math.h>

#ifndef ARRAY_SIZE
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
Expand Down Expand Up @@ -99,6 +100,7 @@ struct stat_specs {
enum stat_id ids[ALL_STATS_CNT];
enum stat_variant variants[ALL_STATS_CNT];
bool asc[ALL_STATS_CNT];
bool abs[ALL_STATS_CNT];
int lens[ALL_STATS_CNT * 3]; /* 3x for comparison mode */
};

Expand Down Expand Up @@ -133,6 +135,7 @@ struct filter {
int stat_id;
enum stat_variant stat_var;
long value;
bool abs;
};

static struct env {
Expand Down Expand Up @@ -455,7 +458,8 @@ static struct {
{ OP_EQ, "=" },
};

static bool parse_stat_id_var(const char *name, size_t len, int *id, enum stat_variant *var);
static bool parse_stat_id_var(const char *name, size_t len, int *id,
enum stat_variant *var, bool *is_abs);

static int append_filter(struct filter **filters, int *cnt, const char *str)
{
Expand Down Expand Up @@ -488,13 +492,14 @@ static int append_filter(struct filter **filters, int *cnt, const char *str)
long val;
const char *end = str;
const char *op_str;
bool is_abs;

op_str = operators[i].op_str;
p = strstr(str, op_str);
if (!p)
continue;

if (!parse_stat_id_var(str, p - str, &id, &var)) {
if (!parse_stat_id_var(str, p - str, &id, &var, &is_abs)) {
fprintf(stderr, "Unrecognized stat name in '%s'!\n", str);
return -EINVAL;
}
Expand Down Expand Up @@ -533,6 +538,7 @@ static int append_filter(struct filter **filters, int *cnt, const char *str)
f->stat_id = id;
f->stat_var = var;
f->op = operators[i].op_kind;
f->abs = true;
f->value = val;

*cnt += 1;
Expand Down Expand Up @@ -657,7 +663,8 @@ static struct stat_def {
[MARK_READ_MAX_LEN] = { "Max mark read length", {"max_mark_read_len", "mark_read"}, },
};

static bool parse_stat_id_var(const char *name, size_t len, int *id, enum stat_variant *var)
static bool parse_stat_id_var(const char *name, size_t len, int *id,
enum stat_variant *var, bool *is_abs)
{
static const char *var_sfxs[] = {
[VARIANT_A] = "_a",
Expand All @@ -667,6 +674,14 @@ static bool parse_stat_id_var(const char *name, size_t len, int *id, enum stat_v
};
int i, j, k;

/* |<stat>| means we take absolute value of given stat */
*is_abs = false;
if (len > 2 && name[0] == '|' && name[len - 1] == '|') {
*is_abs = true;
name += 1;
len -= 2;
}

for (i = 0; i < ARRAY_SIZE(stat_defs); i++) {
struct stat_def *def = &stat_defs[i];
size_t alias_len, sfx_len;
Expand Down Expand Up @@ -722,7 +737,7 @@ static bool is_desc_sym(char c)
static int parse_stat(const char *stat_name, struct stat_specs *specs)
{
int id;
bool has_order = false, is_asc = false;
bool has_order = false, is_asc = false, is_abs = false;
size_t len = strlen(stat_name);
enum stat_variant var;

Expand All @@ -737,14 +752,15 @@ static int parse_stat(const char *stat_name, struct stat_specs *specs)
len -= 1;
}

if (!parse_stat_id_var(stat_name, len, &id, &var)) {
if (!parse_stat_id_var(stat_name, len, &id, &var, &is_abs)) {
fprintf(stderr, "Unrecognized stat name '%s'\n", stat_name);
return -ESRCH;
}

specs->ids[specs->spec_cnt] = id;
specs->variants[specs->spec_cnt] = var;
specs->asc[specs->spec_cnt] = has_order ? is_asc : stat_defs[id].asc_by_default;
specs->abs[specs->spec_cnt] = is_abs;
specs->spec_cnt++;

return 0;
Expand Down Expand Up @@ -1103,7 +1119,7 @@ static int process_obj(const char *filename)
}

static int cmp_stat(const struct verif_stats *s1, const struct verif_stats *s2,
enum stat_id id, bool asc)
enum stat_id id, bool asc, bool abs)
{
int cmp = 0;

Expand All @@ -1124,6 +1140,11 @@ static int cmp_stat(const struct verif_stats *s1, const struct verif_stats *s2,
long v1 = s1->stats[id];
long v2 = s2->stats[id];

if (abs) {
v1 = v1 < 0 ? -v1 : v1;
v2 = v2 < 0 ? -v2 : v2;
}

if (v1 != v2)
cmp = v1 < v2 ? -1 : 1;
break;
Expand All @@ -1142,7 +1163,8 @@ static int cmp_prog_stats(const void *v1, const void *v2)
int i, cmp;

for (i = 0; i < env.sort_spec.spec_cnt; i++) {
cmp = cmp_stat(s1, s2, env.sort_spec.ids[i], env.sort_spec.asc[i]);
cmp = cmp_stat(s1, s2, env.sort_spec.ids[i],
env.sort_spec.asc[i], env.sort_spec.abs[i]);
if (cmp != 0)
return cmp;
}
Expand Down Expand Up @@ -1211,7 +1233,8 @@ static void fetch_join_stat_value(const struct verif_stats_join *s,

static int cmp_join_stat(const struct verif_stats_join *s1,
const struct verif_stats_join *s2,
enum stat_id id, enum stat_variant var, bool asc)
enum stat_id id, enum stat_variant var,
bool asc, bool abs)
{
const char *str1 = NULL, *str2 = NULL;
double v1, v2;
Expand All @@ -1220,6 +1243,11 @@ static int cmp_join_stat(const struct verif_stats_join *s1,
fetch_join_stat_value(s1, id, var, &str1, &v1);
fetch_join_stat_value(s2, id, var, &str2, &v2);

if (abs) {
v1 = fabs(v1);
v2 = fabs(v2);
}

if (str1)
cmp = strcmp(str1, str2);
else if (v1 != v2)
Expand All @@ -1237,7 +1265,8 @@ static int cmp_join_stats(const void *v1, const void *v2)
cmp = cmp_join_stat(s1, s2,
env.sort_spec.ids[i],
env.sort_spec.variants[i],
env.sort_spec.asc[i]);
env.sort_spec.asc[i],
env.sort_spec.abs[i]);
if (cmp != 0)
return cmp;
}
Expand Down Expand Up @@ -1720,6 +1749,9 @@ static bool is_join_stat_filter_matched(struct filter *f, const struct verif_sta

fetch_join_stat_value(stats, f->stat_id, f->stat_var, &str, &value);

if (f->abs)
value = fabs(value);

switch (f->op) {
case OP_EQ: return value > f->value - eps && value < f->value + eps;
case OP_NEQ: return value < f->value - eps || value > f->value + eps;
Expand Down Expand Up @@ -1766,7 +1798,7 @@ static int handle_comparison_mode(void)
struct stat_specs base_specs = {}, comp_specs = {};
struct stat_specs tmp_sort_spec;
enum resfmt cur_fmt;
int err, i, j, last_idx;
int err, i, j, last_idx, cnt;

if (env.filename_cnt != 2) {
fprintf(stderr, "Comparison mode expects exactly two input CSV files!\n\n");
Expand Down Expand Up @@ -1879,7 +1911,7 @@ static int handle_comparison_mode(void)
env.join_stat_cnt += 1;
}

/* now sort joined results accorsing to sort spec */
/* now sort joined results according to sort spec */
qsort(env.join_stats, env.join_stat_cnt, sizeof(*env.join_stats), cmp_join_stats);

/* for human-readable table output we need to do extra pass to
Expand All @@ -1896,16 +1928,22 @@ static int handle_comparison_mode(void)
output_comp_headers(cur_fmt);

last_idx = -1;
cnt = 0;
for (i = 0; i < env.join_stat_cnt; i++) {
const struct verif_stats_join *join = &env.join_stats[i];

if (!should_output_join_stats(join))
continue;

if (env.top_n && cnt >= env.top_n)
break;

if (cur_fmt == RESFMT_TABLE_CALCLEN)
last_idx = i;

output_comp_stats(join, cur_fmt, i == last_idx);

cnt++;
}

if (cur_fmt == RESFMT_TABLE_CALCLEN) {
Expand All @@ -1920,6 +1958,9 @@ static bool is_stat_filter_matched(struct filter *f, const struct verif_stats *s
{
long value = stats->stats[f->stat_id];

if (f->abs)
value = value < 0 ? -value : value;

switch (f->op) {
case OP_EQ: return value == f->value;
case OP_NEQ: return value != f->value;
Expand Down Expand Up @@ -1964,7 +2005,7 @@ static bool should_output_stats(const struct verif_stats *stats)
static void output_prog_stats(void)
{
const struct verif_stats *stats;
int i, last_stat_idx = 0;
int i, last_stat_idx = 0, cnt = 0;

if (env.out_fmt == RESFMT_TABLE) {
/* calculate column widths */
Expand All @@ -1984,7 +2025,10 @@ static void output_prog_stats(void)
stats = &env.prog_stats[i];
if (!should_output_stats(stats))
continue;
if (env.top_n && cnt >= env.top_n)
break;
output_stats(stats, env.out_fmt, i == last_stat_idx);
cnt++;
}
}

Expand Down

0 comments on commit 5d4a7aa

Please sign in to comment.