Hi,
someone noticed that the stats option sometimes gives bogus results. I've checked it and found a bug in tv_sub: it added 1 extra second whenever the two arguments referred the same point in time. While it's possible to fix it, I think it's better to use the already existing timersub and timeradd macros, so I went for that.
The second patch is a bit questionable, it replaces 'nan' in the output with zero. You can decide if it makes sense...
Akos PASZTORY (2): stats: bugfix: use timersub() and timeradd() stats: report zero percents instead of NaN
src/stats.c | 44 ++++++++------------------------------------ 1 files changed, 8 insertions(+), 36 deletions(-)
tv_sub() had a bug: if 'a' represented the same time as 'b' then it added extra 1000000 usecs. --- src/stats.c | 41 +++++------------------------------------ 1 files changed, 5 insertions(+), 36 deletions(-)
diff --git a/src/stats.c b/src/stats.c index b1c8dda..d5c5351 100644 --- a/src/stats.c +++ b/src/stats.c @@ -31,37 +31,6 @@ struct timeval tv_program_start; struct timeval tv_program_stop;
-/* res = a - b*/ -static int tv_sub(struct timeval *res, struct timeval *a, struct timeval *b) -{ - res->tv_sec = a->tv_sec - b->tv_sec; - - if (a->tv_usec > b->tv_usec) - res->tv_usec = a->tv_usec - b->tv_usec; - else { - res->tv_usec = a->tv_usec + (1000000 - b->tv_usec); - if (res->tv_sec) - res->tv_sec--; - } - - return 0; -} - -/* res = a + b*/ -static int tv_add(struct timeval *res, struct timeval *a, struct timeval *b) -{ - struct timeval tv; - tv.tv_sec = a->tv_sec + b->tv_sec; - tv.tv_usec = a->tv_usec + b->tv_usec; - if (tv.tv_usec > 1000000) { - tv.tv_usec -= 1000000; - tv.tv_sec++; - } - - *res = tv; - return 0; -} - int lt_stats_alloc(struct lt_config_app *cfg, struct lt_thread *t) { int i; @@ -186,8 +155,8 @@ int lt_stats_sym(struct lt_config_app *cfg, struct lt_thread *t,
if (FIFO_MSG_TYPE_EXIT == m->h.type) { struct timeval tv; - tv_sub(&tv, &m->h.tv, &sym->tv_cur); - tv_add(&sym->tv_all, &sym->tv_all, &tv); + timersub(&m->h.tv, &sym->tv_cur, &tv); + timeradd(&sym->tv_all, &tv, &sym->tv_all); }
return 0; @@ -248,7 +217,7 @@ static int lt_stats_show_thread(struct lt_config_app *cfg, struct lt_thread *t)
for(i = 0; i < t->sym_cnt; i++) { struct lt_stats_sym *sym = t->sym_array[i]; - tv_add(&tv_thread_accu, &tv_thread_accu, &sym->tv_all); + timeradd(&sym->tv_all, &tv_thread_accu, &tv_thread_accu); }
time_global = tv_thread_accu.tv_sec * 1000000 + tv_thread_accu.tv_usec; @@ -270,7 +239,7 @@ static int lt_stats_show_thread(struct lt_config_app *cfg, struct lt_thread *t)
PRINT_VERBOSE(cfg, 1, "%s\n", "printing");
- tv_sub(&tv_thread_real, &t->tv_stop, &t->tv_start); + timersub(&t->tv_stop, &t->tv_start, &tv_thread_real); printf("\nThread %d (runtime %u.%06u sec)\n", t->tid, (u_int) tv_thread_real.tv_sec, (u_int) tv_thread_real.tv_usec); printf("%3s %-6s %10s %10s %10s %-30s\n", @@ -311,7 +280,7 @@ int lt_stats_show(struct lt_config_app *cfg) struct lt_thread *t;
struct timeval tv_program_real; - tv_sub(&tv_program_real, &tv_program_stop, &tv_program_start); + timersub(&tv_program_stop, &tv_program_start, &tv_program_real);
printf("\n--------------------------------------------------------------------------\n"); printf("Statistics for [%s] total runtime: %u.%06u sec\n",
--- src/stats.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/stats.c b/src/stats.c index d5c5351..410c68e 100644 --- a/src/stats.c +++ b/src/stats.c @@ -24,6 +24,7 @@ #include <stdio.h> #include <stdlib.h> #include <setjmp.h> +#include <math.h>
#include "config.h"
@@ -229,6 +230,8 @@ static int lt_stats_show_thread(struct lt_config_app *cfg, struct lt_thread *t) u_int time_sym = sym->tv_all.tv_sec*1000000 + sym->tv_all.tv_usec;
sym->percent = time_sym / (time_global/100); + if (isnan(sym->percent)) + sym->percent = 0.0f; sym->usec_call = time_sym/sym->call; }
On Sat, Dec 03, 2011 at 03:42:55PM +0000, Akos PASZTORY wrote:
Hi,
someone noticed that the stats option sometimes gives bogus results. I've checked it and found a bug in tv_sub: it added 1 extra second whenever the two arguments referred the same point in time. While it's possible to fix it, I think it's better to use the already existing timersub and timeradd macros, so I went for that.
cool, had no idea those existed :)
The second patch is a bit questionable, it replaces 'nan' in the output with zero. You can decide if it makes sense...
I took both changes.. thanks a lot, jirka
Akos PASZTORY (2): stats: bugfix: use timersub() and timeradd() stats: report zero percents instead of NaN
src/stats.c | 44 ++++++++------------------------------------ 1 files changed, 8 insertions(+), 36 deletions(-)
-- 1.7.7.2
latrace mailing list latrace@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/latrace
latrace@lists.fedorahosted.org