[INN-COMMITTERS] inn/lib (timer.c Makefile)

Fabien Tassin fta at sofaraway.org
Wed Feb 7 17:47:25 UTC 2001


According to Russ Allbery:
> Fabien Tassin <fta at sofaraway.org> writes:
> 
> > argh.. I've rewritten innd/timer.c almost from scratch. The
> > API is the same but the global variables are not.
> > My version can also be used in lib/ to profile both innd and innfeed or
> > even whatever you want at the same time (timer_name and the corresponding
> > enum must then cover them all).
> > I'll send a patch soon but I first need to change innreport a little bit.
> 
> Ugh... sorry about that, I didn't realize you were both working on the
> same thing.  It sounds like you may want to look at Alex's code first,
> though, since it sounds like his approach was a touch more general.

hmm.. for several reasons, I'm not able to sync now but I want to submit
this new timer implementation now.

Here is the patch.. 3 files are impacted.

- 2 to move TMRinit() *before* the first call to TMRstart()
- 1 for */timer.c. If the recent changes are that general, this patch
should not be a problem.

The patch for innreport is not there yet but it is not needed for the
moment. It will be needed when we will begin to use real nested timers
so I can provide it later.

Basically, these nested timers will produce this log line :

TMRinit();
TMRstart(foo);
TMRstart(bar);
TMRstart(baz);
TMRstop(baz);
TMRstop(bar);
TMRstop(foo);
TMRmainloophook();

innd: ME time x foo x1(y1) bar/foo x2(y2) baz/bar/foo x3(y3)

Russ: I've updated some of your comments but not all, especially the
introduction and the size vs nb of timers conciderations in summarize().
As you seem to be concerned by error checking, you can also look at the
FIXME comments bellow.

Concerning a possible usage in a threaded environnement, I think this can
have a sense if we use one "graph" per thread. Concerning the report, the
problem remains.

Last point, no destructor is called. If we have to call one, all is needed
is to add "TMRfree(TMRroot)" somewhere.

Thoughts on this "family" patch ?

--- innd.c.orig Tue Feb  6 13:42:45 2001
+++ innd.c      Wed Feb  7 11:40:02 2001
@@ -448,6 +448,7 @@
        fprintf(stderr, "%s\n", p + 2);
        exit(1);
     }
+    TMRinit(); /* initialize the profiler */
 
     /* Get the Path entry. */
     if (innconf->pathhost == NULL) {


--- chan.c.orig Tue Feb  6 13:42:45 2001
+++ chan.c      Wed Feb  7 11:39:02 2001
@@ -946,7 +946,6 @@
     time_t             LastUpdate;
     HDRCONTENT         *hc;
 
-    TMRinit();
     STATUSinit();
     
     LastUpdate = GetTimeInfo(&Now) < 0 ? 0 : Now.time;


--- timer.c.orig	Tue Feb  6 13:42:46 2001
+++ timer.c	Wed Feb  7 18:45:51 2001
@@ -47,14 +47,29 @@
     "artlog", "datamove"
 };
 
-/* Timer values.  start stores the time (relative to the last summary) at
-   which TMRstart was last called for each timer, or 0 if the timer hasn't
-   been started.  cumulative is the total time accrued by that timer since
-   the last summary.  count is the number of times the timer has been
-   stopped since the last summary. */
-static unsigned long start[TMR_MAX];
-static unsigned long cumulative[TMR_MAX];
-static unsigned long count[TMR_MAX];
+/* Timer values are stored into a simple graph. This allows use to use
+   nested timers. Each node (ie timer) is linked to three of its
+   neighbours to make lookups easy and fast. The root of the graph is
+   TMRroot and the current position into this graph is given by TMRcur.
+   Note that without the 'father' pointer, this oriented graph is a tree.
+   id is the identifier of the timer. start stores the time (relative to
+   the last summary) at which TMRstart was last called for each timer.
+   cumulative is the total time accrued by that timer since the last
+   summary. count is the number of times the timer has been stopped since
+   the last summary. */
+
+typedef struct _TMRnode_t TMRnode_t;
+
+struct _TMRnode_t {
+  enum timer     id;
+  unsigned long  start;
+  unsigned long  cumulative;
+  unsigned long  count;
+  TMRnode_t     *father;
+  TMRnode_t     *brother;
+  TMRnode_t     *child;
+};
+static TMRnode_t *TMRroot = NULL, *TMRcur = NULL;
 
 /* The time of the last summary, used as a base for times returned by
    get_time.  Formerly, times were relative to the last call to TMRinit,
@@ -64,6 +79,18 @@
    approach is less confusing to follow. */
 static struct timeval base;
 
+/* Destroy the structure, recursively */
+static void
+TMRfree(TMRnode_t *node)
+{
+    if (node == NULL)
+        return;
+    if (node->child != NULL)
+        TMRfree(node->child);
+    if (node->brother != NULL)
+        TMRfree(node->brother);
+    DISPOSE(node);
+}
 
 /*
 **  Returns the number of milliseconds since the base time.  This gives
@@ -85,6 +112,32 @@
     return now;
 }
 
+static void
+TMRsumone(TMRnode_t *node, char *buffer, int *pos)
+{
+    char result[256];
+    int offs = 0;
+    TMRnode_t *n;
+
+    n = node;
+    while (n->father != NULL) {
+        offs += sprintf(result + offs, "%s/", timer_name[n->id]);
+        n = n->father;
+    }
+    offs += sprintf(result + offs - 1, " %lu(%lu) ", node->cumulative,
+		    node->count);
+    node->cumulative = node->count = 0;
+    if (*pos + offs > 2048) {
+        warn("FIXME: Timer log line too long. Max length of 2048 reached");
+        return;
+    }
+    strcat(buffer + *pos, result);
+    *pos += offs - 1;
+    if (node->child != NULL)
+        TMRsumone(node->child, buffer, pos);
+    if (node->brother != NULL)
+        TMRsumone(node->brother, buffer, pos);
+}
 
 /*
 **  Summarize the current timer statistics and then reset them for the next
@@ -98,22 +151,12 @@
 summarize(void)
 {
     char buffer[2048];
-    char result[256];
-    int i;
-    int length = SIZEOF(buffer) - 1;
+    int  pos;
 
-    length -= sprintf(buffer, "ME time %ld ", get_time(true));
-    for (i = 0; i < TMR_MAX; i++) {
-	if (cumulative[i] != 0 || count[i] != 0) {
-	    length -= sprintf(result, "%s %lu(%lu) ", timer_name[i],
-			      cumulative[i], count[i]);
-	    if (length < 0)
-		break;
-	    strcat(buffer, result);
-	    cumulative[i] = 0;
-	    count[i] = 0;
-        }
-    }
+    if (TMRroot->child == NULL)
+        return; /* this should not happen */
+    pos = sprintf(buffer, "ME time %ld ", get_time(true));
+    TMRsumone(TMRroot->child, buffer, &pos);
     syslog(LOG_NOTICE, "%s", buffer);
 }
 
@@ -127,14 +170,13 @@
 {
     int i;
 
+    if (TMRroot != NULL) /* free the structure */
+        TMRfree(TMRroot);
+    TMRroot = TMRcur = NEW(TMRnode_t, 1);
+    TMRroot->id = TMR_MAX;
+    TMRroot->father = TMRroot->brother = TMRroot->child = NULL;
     /* Establish a base time. */
     get_time(true);
-
-    for (i = 0; i < TMR_MAX; i++) {
-        start[i] = 0;
-        cumulative[i] = 0;
-	count[i] = 0;
-    }
 }
 
 
@@ -169,9 +211,35 @@
 void
 TMRstart(enum timer timer)
 {
+    TMRnode_t *father;
+
     if (!innconf->timer)
 	return;
-    start[timer] = get_time(false);
+
+    father = TMRcur;
+    /* go to the "child" level and look for the good "brother" (ie timer) */
+    if (TMRcur->child == NULL) {
+        TMRcur->child = NEW(TMRnode_t, 1);
+        TMRcur = TMRcur->child;
+        TMRcur->father = father;
+        TMRcur->brother = TMRcur->child = NULL;
+        TMRcur->id = timer;
+	TMRcur->cumulative = TMRcur->count = 0;
+    }
+    else {
+        TMRcur = TMRcur->child;
+        while (TMRcur->id != timer && TMRcur->brother != NULL)
+	    TMRcur = TMRcur->brother;
+        if (TMRcur->id != timer) { /* not found */
+	    TMRcur->brother = NEW(TMRnode_t, 1);
+	    TMRcur = TMRcur->brother;
+	    TMRcur->father = father;
+    	    TMRcur->brother = TMRcur->child = NULL;
+	    TMRcur->id = timer;
+	    TMRcur->cumulative = TMRcur->count = 0;
+        }
+    }
+    TMRcur->start = get_time(false);
 }
 
 
@@ -184,6 +252,14 @@
 {
     if (!innconf->timer)
 	return;
-    cumulative[timer] += get_time(false) - start[timer];
-    count[timer]++;
+
+    /* perform a closing timer type check */
+    if (timer != TMRcur->id) {
+        warn("Warning: closing timer '%s' does not match '%s'. Investigate.",
+	     timer_name[timer], timer_name[TMRcur->id]);
+        /* FIXME: what should we do here ? die ? */
+    }
+    TMRcur->cumulative += get_time(false) - TMRcur->start;
+    TMRcur->count++;
+    TMRcur = TMRcur->father;
 }

-- 
Fabien Tassin -+- fta at sofaraway.org


More information about the inn-workers mailing list