[sheepdog] [PATCH v2 4/5] collie/trace: add graph stat function
MORITA Kazutaka
morita.kazutaka at gmail.com
Sun Aug 11 22:04:12 CEST 2013
> +
> +static void stat_list_print(void)
> +{
> + struct graph_stat_entry *entry;
> +
> + list_for_each_entry(entry, &stat_list, list) {
> + float f = (float)entry->duration / 1000000000;
> + float pf = (float)entry->duration / entry->nr_calls / 1000;
> +
> + printf("%-*s| %-10"PRIu32" | %-16f| %-16f\n", TRACE_FNAME_LEN,
> + entry->fname, entry->nr_calls, f, pf);
I prefer a right-aligned number. In addition, the default number of
digits after the decimal point looks long to me. Then the format
string is like "%-*s| %10"PRIu32" |%16.3f |%16.3f \n".
I wonder if we can use the same format as the existing tool's one?
E.g. gprof output is
http://sourceware.org/binutils/docs/gprof/Flat-Profile.html#Flat-Profile
I think it'd be great if we can have the "self seconds" column to
investigate a bottleneck.
> + }
> +}
> +
> +static int stat_list_cmp(void *priv, struct list_head *a, struct list_head *b)
> +{
> + struct graph_stat_entry *ga = container_of(a, struct graph_stat_entry,
> + list);
> + struct graph_stat_entry *gb = container_of(b, struct graph_stat_entry,
> + list);
> +
> + if (ga->duration == gb->duration)
> + return 0;
> + else if (ga->duration > gb->duration)
> + return -1; /* This is for reverse sort, largest first */
> + else
> + return 1;
Using intcmp looks simpler.
> +}
> +
> +static void stat_trace_file(void *buf, size_t size)
> +{
> + struct trace_graph_item *item = (struct trace_graph_item *)buf;
> + size_t sz = size / sizeof(struct trace_graph_item), i;
> +
> + printf(" Function Name | Call NR | Total time(s) | Time Per Call(us) \n");
> + printf("--------------------------------------------------------------------------------\n");
Too long line.
Grammatically speaking, I think spaces are needed between "Total
time" and "(s)" and between "Time Per Call" and "(us)"
Thanks,
Kazutaka
More information about the sheepdog
mailing list