[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