From: Nick Mathewson <nickm@torproject.org>
Date: Mon, 9 Nov 2009 19:37:27 +0000 (+0000)
Subject: Change event_base.activequeues to "array of eventlist".
X-Git-Tag: release-2.0.3-alpha~23
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=74871cacb8318116e2cf24958e1456e9b429542b;p=libevent

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 c1a6e5ec..4834f424 100644
--- 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 4fef73c1..68329761 100644
--- 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 8f340a8e..ec91f8e9 100644
--- 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) {