[asterisk-commits] murf: branch murf/bug8574-1.2 r51276 - /team/murf/bug8574-1.2/pbx.c

asterisk-commits at lists.digium.com asterisk-commits at lists.digium.com
Thu Jan 18 20:19:18 MST 2007


Author: murf
Date: Thu Jan 18 21:19:17 2007
New Revision: 51276

URL: http://svn.digium.com/view/asterisk?view=rev&rev=51276
Log:
First attempt at rearranging the merge algorithm to avoid locking up stuff too long with big dialplans

Modified:
    team/murf/bug8574-1.2/pbx.c

Modified: team/murf/bug8574-1.2/pbx.c
URL: http://svn.digium.com/view/asterisk/team/murf/bug8574-1.2/pbx.c?view=diff&rev=51276&r1=51275&r2=51276
==============================================================================
--- team/murf/bug8574-1.2/pbx.c (original)
+++ team/murf/bug8574-1.2/pbx.c Thu Jan 18 21:19:17 2007
@@ -548,6 +548,7 @@
 
 static struct ast_context *contexts = NULL;
 AST_MUTEX_DEFINE_STATIC(conlock); 		/* Lock for the ast_context list */
+AST_MUTEX_DEFINE_STATIC(conreloadlock); 		/* Lock for the ast_context list; to keep modding threads from disturbing your reload */
 static struct ast_app *apps = NULL;
 AST_MUTEX_DEFINE_STATIC(applock); 		/* Lock for the application list */
 
@@ -3685,6 +3686,7 @@
 	length += strlen(name) + 1;
 	if (!extcontexts) {
 		local_contexts = &contexts;
+		ast_mutex_lock(&conreloadlock); /* don't stick this in while we're reloading */
 		ast_mutex_lock(&conlock);
 	} else
 		local_contexts = extcontexts;
@@ -3693,8 +3695,10 @@
 	while(tmp) {                              /* LINEAR SEARCH FOR CONTEXT */
 		if (!strcasecmp(tmp->name, name)) {
 			ast_log(LOG_WARNING, "Tried to register context '%s', already in use\n", name);
-			if (!extcontexts)
+			if (!extcontexts) {
 				ast_mutex_unlock(&conlock);
+				ast_mutex_unlock(&conreloadlock);
+			}
 			return NULL;
 		}
 		tmp = tmp->next;
@@ -3717,8 +3721,11 @@
 	} else
 		ast_log(LOG_ERROR, "Out of memory\n");
 	
-	if (!extcontexts)
+	if (!extcontexts) {
 		ast_mutex_unlock(&conlock);
+		ast_mutex_unlock(&conreloadlock);
+	}
+	
 	return tmp;
 }
 
@@ -3733,6 +3740,92 @@
 	char data[1];
 };
 
+static void separate_sheep_from_goats(int *nonregistrar_count, int *registrar_count, struct ast_context ***nonregistrar_list, struct ast_context ***registrar_list, const char *registrar);
+
+static void separate_sheep_from_goats(int *nonregistrar_count, int *registrar_count, struct ast_context ***nonregistrar_list, struct ast_context ***registrar_list, const char *registrar)
+{
+	/* think of the sheep as the contexts we will keep, because their registrar entry doesn't match the one set by the reloader, and won't be deleted.
+	   think of the goats as the contexts that are reloaded, whose registrars match that specified by the loader, and will be deleted.
+	   So, the sheep are the nonregistrar_list, and the goats are the registrar_list.
+	   Pass in the address of two ints to get the size of each array. And, the addresses of two pointers, which this routine will set to point
+	   to the two arrays malloc'd by this routine. Don't forget to free these arrays when you are done! 
+	*/
+	int sheep_count = 0, goat_count = 0;
+	int s = 0, g=0;
+	struct ast_context *tmp=NULL, **sheep, **goats;
+	tmp = contexts;
+	while(tmp) {
+		if (strcasecmp(registrar, tmp->registrar) == 0)
+			goat_count++;
+		else
+			sheep_count++;
+		tmp = tmp->next;
+	}
+	*nonregistrar_count = sheep_count;
+	*registrar_count = goat_count;
+	*nonregistrar_list = sheep = calloc(sheep_count,sizeof(struct ast_context *));
+	*registrar_list = goats = calloc(goat_count,sizeof(struct ast_context *));
+	/* now, go thru the list again, and fill up those arrays */
+	tmp = contexts;
+	while(tmp) {
+		if (strcasecmp(registrar, tmp->registrar) == 0) {
+			goats[g++] = tmp;
+		} else {
+			sheep[s++] = tmp;
+		}
+		tmp = tmp->next;
+	}
+}
+
+static void destroy_context(struct ast_context *con);
+static void destroy_exten(struct ast_exten *e);
+
+static void destroy_context(struct ast_context *con) /* destroy a non-linked context */
+{
+	struct ast_include *coni, *conil= NULL;
+	struct ast_sw *sw, *swl= NULL;
+	struct ast_exten *e, *el, *en;
+	struct ast_ignorepat *ipi, *ipl = NULL;
+
+	if (ast_mutex_lock(&con->lock)) { /* we do this just to make sure nobody's been caught in here traversing */
+		ast_log(LOG_WARNING, "Unable to lock context lock\n");
+		return;
+	}
+	ast_mutex_unlock(&con->lock);
+	for (coni = con->includes; coni; ) {
+		/* Free includes */
+		conil = coni;
+		coni = coni->next;
+		free(conil);
+	}
+	for (ipi = con->ignorepats; ipi; ) {
+		/* Free ignorepats */
+		ipl = ipi;
+		ipi = ipi->next;
+		free(ipl);
+	}
+	for (sw = con->alts; sw; ) {
+		/* Free switches */
+		swl = sw;
+		sw = sw->next;
+		free(swl);
+		swl = sw;
+	}
+	for (e = con->root; e;) {
+		for (en = e->peer; en;) {
+			el = en;
+			en = en->peer;
+			destroy_exten(el);
+		}
+		el = e;
+		e = e->next;
+		destroy_exten(el);
+	}
+	ast_mutex_destroy(&con->lock);
+	free(con);
+}
+
+
 AST_LIST_HEAD(store_hints, store_hint);
 
 void ast_merge_contexts_and_delete(struct ast_context **extcontexts, const char *registrar)
@@ -3742,14 +3835,68 @@
 	struct store_hint *this;
 	struct ast_hint *hint;
 	struct ast_exten *exten;
-	int length;
+	int length,i;
 	struct ast_state_cb *thiscb, *prevcb;
+	int registrar_match_count;
+	int registrar_nonmatch_count;
+	struct ast_context **registrar_match_array;
+	struct ast_context **registrar_nonmatch_array;
 	struct timeval begin, end, diff;
 	struct timeval begin2, end2, diff2;
 
 	memset(&store, 0, sizeof(store));
 	AST_LIST_HEAD_INIT(&store);
-
+	
+	ast_mutex_lock(&conreloadlock);
+
+	/* while this lock is in place, all other context-modifiers should be held off. 
+	   But, from now until the conlock is achieved, all list traversers (hopefully the 
+	   majority of threads) can do their thing as usual.
+	   In this brief slice of time, we want to do whatever is necessary to make the time we 
+	   hold the conlock as brief as possible:
+      ALGORITHM 1:
+	  Duplicate the context list, and then do all operations on the list that used
+	  to be done with the conlock set. (remove all the matching registrar'd items, take
+      the remainder and add them to the new context list), but in this case, we won't need to lock
+	  anything while all of this is done  Then, simply get the lock, and swap pointers, and you are done.
+	  Drop the conlock.  Now, delete all the contexts in the old list. Drop the conreloadlock.
+	  (You only need to dup the contexts themselves; whatever pointers are in the context can simply be 
+	  copied, leaving all the substructure the same.)
+	  (But, if you take this short-cut, what do you destroy when you free a context? -- Some will be have
+      pointers to substructure which are used in the new context list, some will not!)
+	  Advantages: Least amnt of time with conlock set. Fixed time in conlock.
+	  Disadvantages: Time to completely rebuild the context list, and then destroy most of it.
+
+	  ALGORITHM 2: 
+	  Count the contexts that would be destroyed via the registrar matching, and alloc an array, and
+	  store the pointers to those contexts in that array. Count and alloc another array for all those that do not
+	  match registrars. Get a pointer to the last context in the new list of contexts. 
+	  Then, get the conlock. Make it point to the new list of contexts. Then, thread all the non-matching registrard
+	  contexts to the end of the new list. Drop the conlock. Destroy all the contexts in matched-registrar list. You are
+	  finished. Drop the conreloadlock.
+	  Advantages: A little less memory and time intensive. No dups of context take place.
+	  Disadvantages: conlock time is a function of how many contexts that will not be deleted in the reload.
+	                 (but, hopefully this is a minority in a large dialplan, and all we are doing is setting
+					 one pointer per context in this case, so it should be quick)
+
+		 On the first pass, we will attempt to implement algorithm 2.
+
+	*/
+	begin2 = ast_tvnow();
+	separate_sheep_from_goats(&registrar_nonmatch_count, &registrar_match_count, 
+							  &registrar_nonmatch_array, &registrar_match_array,
+							  registrar);
+	
+	tmp = *extcontexts;
+	while (tmp) {
+		lasttmp = tmp;
+		tmp = tmp->next;
+	}
+	
+	end2 = ast_tvnow();
+	diff2 = ast_tvdiff(end2, begin2);
+	ast_log(LOG_NOTICE, "---time spent after conreloadlock was set, before the conlock was set was %d.%06d sec\n", (int)diff2.tv_sec, (int)diff2.tv_usec);
+	
 	/* it is very important that this function hold the hintlock _and_ the conlock
 	   during its operation; not only do we need to ensure that the list of contexts
 	   and extensions does not change, but also that no hint callbacks (watchers) are
@@ -3764,7 +3911,7 @@
 	end = ast_tvnow();
 	diff = ast_tvdiff(end, begin);
 	conlock_gettime += ((int)diff.tv_sec*1000000) + (int)diff.tv_usec;
-
+	
 	/* preserve all watchers for hints associated with this registrar */
 	for (hint = hints; hint; hint = hint->next) {
 		if (hint->callbacks && !strcmp(registrar, hint->exten->parent->registrar)) {
@@ -3785,6 +3932,7 @@
 		}
 	}
 
+#ifdef OLD_STUFF
 	tmp = *extcontexts;
 	if (registrar) {
 		begin2 = ast_tvnow();
@@ -3803,13 +3951,26 @@
 			tmp = tmp->next;
 		}
 	}
+#endif
+	
+	begin2 = ast_tvnow();
 	if (lasttmp) {
-		lasttmp->next = contexts; /* add to the last context any that might remain in the system contexts */
+		int i=0;
+		while (i < registrar_nonmatch_count)  /* if this is not fast enough, then we have to go to algorithm 1 */
+		{
+			lasttmp->next = registrar_nonmatch_array[i++];
+			lasttmp = lasttmp->next;
+		}
+		lasttmp->next = 0; /* end the list */
+		
 		contexts = *extcontexts;
 		*extcontexts = NULL;
 	} else 
 		ast_log(LOG_WARNING, "Requested contexts didn't get merged\n");
 
+	end2 = ast_tvnow();
+	diff2 = ast_tvdiff(end2, begin2);
+	ast_log(LOG_NOTICE, "---time spent relinking in new contexts in conlock was %d.%06d sec\n", (int)diff2.tv_sec, (int)diff2.tv_usec);
 	/* restore the watchers for hints that can be found; notify those that
 	   cannot be restored
 	*/
@@ -3851,6 +4012,19 @@
 	diff = ast_tvdiff(end, begin);
 	conlock_total_holdtime += ((int)diff.tv_sec*1000000) + (int)diff.tv_usec;
 	conlock_total_sets++;
+
+	/* ok, now, at our leisure, we can now delete all the contexts in the matched list;
+	   we won't need them, and they aren't pointed at anymore. */
+	begin2 = ast_tvnow();
+	for (i=0;i<registrar_match_count;i++)
+	{
+		destroy_context(registrar_match_array[i]);
+	}
+	free(registrar_match_array);
+	free(registrar_nonmatch_array);
+	end2 = ast_tvnow();
+	diff2 = ast_tvdiff(end2, begin2);
+	ast_log(LOG_NOTICE, "---time spent in destroying old contexts (%d) was %d.%06d sec\n", registrar_match_count, (int)diff2.tv_sec, (int)diff2.tv_usec);
 	return;	
 }
 
@@ -6335,12 +6509,14 @@
  */
 int ast_lock_contexts()
 {
+	ast_mutex_lock(&conreloadlock);
 	return ast_mutex_lock(&conlock);
 }
 
 int ast_unlock_contexts()
 {
-	return ast_mutex_unlock(&conlock);
+	ast_mutex_unlock(&conlock);
+	return ast_mutex_unlock(&conreloadlock);
 }
 
 /*



More information about the asterisk-commits mailing list