Change event_base.activequeues to "array of eventlist".

Previously, event_base.activequeues was of type "array of pointers to
eventlist."  This was pointless: none of the eventlists were allowed
to be NULL.  Worse, it was inefficient:

  - It made looking up an active event queue take two pointer
    deferences instead of one, thus risking extra cache misses.
  - It used more RAM than it needed to, because of the extra pointer
    and the malloc overhead.

Also, this patch fixes a bug where we were saying
calloc(N,N*sizeof(X)) instead of calloc(N,sizeof(X)) when allocating
activequeues.  That part, I'll backport.

Also, we warn and return -1 on failure to allocate activequeues,
rather than calling event_err.

svn:r1525

diff --git a/ChangeLog b/ChangeLog
index c1a6e5ec20d3ec516069565ad3757d3db5366a62..4834f424e28883b52b7c804512740b8b076fcc24 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -43,6 +43,7 @@ Changes in 2.0.3-alpha:
  o New event_base_config option to disable the timeval cache entirely.
  o Make EV_PERSIST timeouts more accurate: schedule the next event based on the scheduled time of the previous event, not based on the current time.
  o Allow http.c to handle cases where getaddrinfo returns an IPv6 address.  Patch from Ryan Phillips.
+ o Fix a problem with excessive memory allocation when using multiple event priorities.
 
 
 Changes in 2.0.2-alpha:

diff --git a/event-internal.h b/event-internal.h
index 4fef73c13f42d5c6c0c186b37fcdac92bf4d4ebf..683297614eea1c5f03a69fc24f967f26c93d9f5d 100644 (file)
--- a/event-internal.h
+++ b/event-internal.h
@@ -138,7 +138,7 @@ struct event_base {
         * have triggered, and whose callbacks need to be called).  Low
         * priority numbers are more important, and stall higher ones.
         */
-       struct event_list **activequeues;
+       struct event_list *activequeues;
        int nactivequeues;
 
        struct common_timeout_list **common_timeout_queues;

diff --git a/event.c b/event.c
index 8f340a8eedf2b0913841317ad12a97a25ca34c9c..ec91f8e9b1beb7d3e1eb1dbb598ab0aaf966974e 100644 (file)
--- a/event.c
+++ b/event.c
@@ -338,7 +338,10 @@ event_base_new_with_config(struct event_config *cfg)
                event_msgx("libevent using: %s", base->evsel->name);
 
        /* allocate a single active event queue */
-       event_base_priority_init(base, 1);
+       if (event_base_priority_init(base, 1) < 0) {
+               event_base_free(base);
+               return NULL;
+       }
 
        /* prepare for threading */
        base->th_notify_fd[0] = -1;
@@ -438,7 +441,7 @@ event_base_free(struct event_base *base)
                mm_free(base->common_timeout_queues);
 
        for (i = 0; i < base->nactivequeues; ++i) {
-               for (ev = TAILQ_FIRST(base->activequeues[i]); ev; ) {
+               for (ev = TAILQ_FIRST(&base->activequeues[i]); ev; ) {
                        struct event *next = TAILQ_NEXT(ev, ev_active_next);
                        if (!(ev->ev_flags & EVLIST_INTERNAL)) {
                                event_del(ev);
@@ -456,13 +459,11 @@ event_base_free(struct event_base *base)
                base->evsel->dealloc(base);
 
        for (i = 0; i < base->nactivequeues; ++i)
-               EVUTIL_ASSERT(TAILQ_EMPTY(base->activequeues[i]));
+               EVUTIL_ASSERT(TAILQ_EMPTY(&base->activequeues[i]));
 
        EVUTIL_ASSERT(min_heap_empty(&base->timeheap));
        min_heap_dtor(&base->timeheap);
 
-       for (i = 0; i < base->nactivequeues; ++i)
-               mm_free(base->activequeues[i]);
        mm_free(base->activequeues);
 
        EVUTIL_ASSERT(TAILQ_EMPTY(&base->eventqueue));
@@ -645,25 +646,21 @@ event_base_priority_init(struct event_base *base, int npriorities)
                return (0);
 
        if (base->nactivequeues) {
-               for (i = 0; i < base->nactivequeues; ++i) {
-                       mm_free(base->activequeues[i]);
-               }
                mm_free(base->activequeues);
+               base->nactivequeues = 0;
        }
 
        /* Allocate our priority queues */
+       base->activequeues = (struct event_list *)
+         mm_calloc(npriorities, sizeof(struct event_list));
+       if (base->activequeues == NULL) {
+               event_warn("%s: calloc", __func__);
+               return (-1);
+       }
        base->nactivequeues = npriorities;
-       base->activequeues = (struct event_list **)mm_calloc(
-           base->nactivequeues,
-           npriorities * sizeof(struct event_list *));
-       if (base->activequeues == NULL)
-               event_err(1, "%s: calloc", __func__);
-
+                               
        for (i = 0; i < base->nactivequeues; ++i) {
-               base->activequeues[i] = mm_malloc(sizeof(struct event_list));
-               if (base->activequeues[i] == NULL)
-                       event_err(1, "%s: malloc", __func__);
-               TAILQ_INIT(base->activequeues[i]);
+               TAILQ_INIT(&base->activequeues[i]);
        }
 
        return (0);
@@ -974,8 +971,8 @@ event_process_active(struct event_base *base)
        int i, c;
 
        for (i = 0; i < base->nactivequeues; ++i) {
-               if (TAILQ_FIRST(base->activequeues[i]) != NULL) {
-                       activeq = base->activequeues[i];
+               if (TAILQ_FIRST(&base->activequeues[i]) != NULL) {
+                       activeq = &base->activequeues[i];
                        c = event_process_active_single_queue(base, activeq);
                        if (c < 0)
                                return;
@@ -1899,7 +1896,7 @@ event_queue_remove(struct event_base *base, struct event *ev, int queue)
                break;
        case EVLIST_ACTIVE:
                base->event_count_active--;
-               TAILQ_REMOVE(base->activequeues[ev->ev_pri],
+               TAILQ_REMOVE(&base->activequeues[ev->ev_pri],
                    ev, ev_active_next);
                break;
        case EVLIST_TIMEOUT:
@@ -1962,7 +1959,7 @@ event_queue_insert(struct event_base *base, struct event *ev, int queue)
                break;
        case EVLIST_ACTIVE:
                base->event_count_active++;
-               TAILQ_INSERT_TAIL(base->activequeues[ev->ev_pri],
+               TAILQ_INSERT_TAIL(&base->activequeues[ev->ev_pri],
                    ev,ev_active_next);
                break;
        case EVLIST_TIMEOUT: {
@@ -2212,7 +2209,7 @@ event_base_dump_events(struct event_base *base, FILE *output)
 
        }
        for (i = 0; i < base->nactivequeues; ++i) {
-               if (TAILQ_EMPTY(base->activequeues[i]))
+               if (TAILQ_EMPTY(&base->activequeues[i]))
                        continue;
                fprintf(output, "Active events [priority %d]:\n", i);
                TAILQ_FOREACH(e, &base->eventqueue, ev_next) {