main/pbx: Improve performance of dialplan reloads with a large number of hints

The PBX core maintains two hash tables for hints: a container of the
actual hints (hints), along with a container of devices that are watching that
hint (hintdevices). When a dialplan reload occurs, each hint in the hints
container is destroyed; this requires a lookup in the container of devices to
find the device => hint mapping object. In the current code, this performs an
ao2_callback, iterating over each of the device to hint objects in the
hintdevices container. For a large number of hints, this is extremely
expensive: dialplan reloads with 20000 hints could take several minutes
in just this phase.

This patch improves the performance of this step in the dialplan reloads
by caching which devices are watching a hint on the hint object itself.
Since we don't want to create a circular reference, we just cache the
name of the device. This allows us to perform a smarter ao2_callback on
the hintdevices container during hint removal, hashing on the name of the
device and returning an iterator to the matching names. The overall
performance improvement is rather large, taking this step down to a number of
seconds as opposed to minutes.

In addition, this patch also registers the hint containers in the PBX
core with the astobj2 library. This allows for reasonable debugging to
hash collisions in those containers.

ASTERISK-25040 #close
Reported by: Matt Jordan

Change-Id: Iedfc97a69d21070c50fca42275d7b3e714e59360

1 files changed, 130 insertions, 19 deletions

diff --git a/main/pbx.c b/main/pbx.c
index fee4191aa..45909f5d9 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -72,6 +72,7 @@ ASTERISK_REGISTER_FILE()
 #include "asterisk/astobj2.h"
 #include "asterisk/stasis_channels.h"
 #include "asterisk/dial.h"
+#include "asterisk/vector.h"
 
 /*!
  * \note I M P O R T A N T :
@@ -1046,8 +1047,9 @@ struct ast_hint {
 
 	char context_name[AST_MAX_CONTEXT];/*!< Context of destroyed hint extension. */
 	char exten_name[AST_MAX_EXTENSION];/*!< Extension of destroyed hint extension. */
-};
 
+	AST_VECTOR(, char *) devices; /*!< Devices associated with the hint */
+};
 
 #define HINTDEVICE_DATA_LENGTH 16
 AST_THREADSTORAGE(hintdevice_data);
@@ -1077,15 +1079,28 @@ struct ast_hintdevice {
 	char hintdevice[1];
 };
 
-
 /*!
  * \note Using the device for hash
  */
 static int hintdevice_hash_cb(const void *obj, const int flags)
 {
-	const struct ast_hintdevice *ext = obj;
+	const struct ast_hintdevice *ext;
+	const char *key;
 
-	return ast_str_case_hash(ext->hintdevice);
+	switch (flags & OBJ_SEARCH_MASK) {
+	case OBJ_SEARCH_KEY:
+		key = obj;
+		break;
+	case OBJ_SEARCH_OBJECT:
+		ext = obj;
+		key = ext->hintdevice;
+		break;
+	default:
+		ast_assert(0);
+		return 0;
+	}
+
+	return ast_str_case_hash(key);
 }
 /*!
  * \note Devices on hints are not unique so no CMP_STOP is returned
@@ -1094,29 +1109,58 @@ static int hintdevice_hash_cb(const void *obj, const int flags)
  */
 static int hintdevice_cmp_multiple(void *obj, void *arg, int flags)
 {
-	struct ast_hintdevice *ext = obj, *ext2 = arg;
+	struct ast_hintdevice *left = obj;
+	struct ast_hintdevice *right = arg;
+	const char *right_key = arg;
+	int cmp;
 
-	return !strcasecmp(ext->hintdevice, ext2->hintdevice) ? CMP_MATCH  : 0;
+	switch (flags & OBJ_SEARCH_MASK) {
+	case OBJ_SEARCH_OBJECT:
+		right_key = right->hintdevice;
+		/* Fall through */
+	case OBJ_SEARCH_KEY:
+		cmp = strcmp(left->hintdevice, right_key);
+		break;
+	case OBJ_SEARCH_PARTIAL_KEY:
+		/*
+		* We could also use a partial key struct containing a length
+		* so strlen() does not get called for every comparison instead.
+		*/
+		cmp = strncmp(left->hintdevice, right_key, strlen(right_key));
+		break;
+	default:
+		ast_assert(0);
+		cmp = 0;
+		break;
+	}
+	return cmp ? 0 : CMP_MATCH;
 }
 
-/*
- * \details This is used with ao2_callback to remove old devices
- * when they are linked to the hint
-*/
-static int hintdevice_remove_cb(void *deviceobj, void *arg, int flags)
+/*! \internal \brief \c ao2_callback function to remove hintdevices */
+static int hintdevice_remove_cb(void *obj, void *arg, void *data, int flags)
 {
-	struct ast_hintdevice *device = deviceobj;
-	struct ast_hint *hint = arg;
+	struct ast_hintdevice *candidate = obj;
+	char *device = arg;
+	struct ast_hint *hint = data;
 
-	return (device->hint == hint) ? CMP_MATCH : 0;
+	if (!strcmp(candidate->hintdevice, device)
+		&& candidate->hint == hint) {
+		return CMP_MATCH;
+	}
+	return 0;
 }
 
 static int remove_hintdevice(struct ast_hint *hint)
 {
-	/* iterate through all devices and remove the devices which are linked to this hint */
-	ao2_t_callback(hintdevices, OBJ_NODATA | OBJ_MULTIPLE | OBJ_UNLINK,
-		hintdevice_remove_cb, hint,
-		"callback to remove all devices which are linked to a hint");
+	while (AST_VECTOR_SIZE(&hint->devices) > 0) {
+		char *device = AST_VECTOR_GET(&hint->devices, 0);
+
+		ao2_t_callback_data(hintdevices, OBJ_SEARCH_KEY | OBJ_UNLINK | OBJ_NODATA,
+			hintdevice_remove_cb, device, hint, "Remove device from container");
+		AST_VECTOR_REMOVE_UNORDERED(&hint->devices, 0);
+		ast_free(device);
+	}
+
 	return 0;
 }
 
@@ -1161,18 +1205,32 @@ static int add_hintdevice(struct ast_hint *hint, const char *devicelist)
 
 	/* Spit on '&' and ',' to handle presence hints as well */
 	while ((cur = strsep(&parse, "&,"))) {
+		char *device_name;
+
 		devicelength = strlen(cur);
 		if (!devicelength) {
 			continue;
 		}
+
+		device_name = ast_strdup(cur);
+		if (!device_name) {
+			return -1;
+		}
+
 		device = ao2_t_alloc(sizeof(*device) + devicelength, hintdevice_destroy,
 			"allocating a hintdevice structure");
 		if (!device) {
+			ast_free(device_name);
 			return -1;
 		}
 		strcpy(device->hintdevice, cur);
 		ao2_ref(hint, +1);
 		device->hint = hint;
+		if (AST_VECTOR_APPEND(&hint->devices, device_name)) {
+			ast_free(device_name);
+			ao2_ref(device, -1);
+			return -1;
+		}
 		ao2_t_link(hintdevices, device, "Linking device into hintdevice container.");
 		ao2_t_ref(device, -1, "hintdevice is linked so we can unref");
 	}
@@ -5385,7 +5443,7 @@ static void device_state_cb(void *unused, struct stasis_subscription *sub, struc
 
 	ast_mutex_lock(&context_merge_lock);/* Hold off ast_merge_contexts_and_delete */
 	dev_iter = ao2_t_callback(hintdevices,
-		OBJ_POINTER | OBJ_MULTIPLE,
+		OBJ_SEARCH_OBJECT | OBJ_MULTIPLE,
 		hintdevice_cmp_multiple,
 		cmpdevice,
 		"find devices in container");
@@ -5697,6 +5755,7 @@ static int hint_id_cmp(void *obj, void *arg, int flags)
 static void destroy_hint(void *obj)
 {
 	struct ast_hint *hint = obj;
+	int i;
 
 	if (hint->callbacks) {
 		struct ast_state_cb *state_cb;
@@ -5726,6 +5785,12 @@ static void destroy_hint(void *obj)
 		}
 		ao2_ref(hint->callbacks, -1);
 	}
+
+	for (i = 0; i < AST_VECTOR_SIZE(&hint->devices); i++) {
+		char *device = AST_VECTOR_GET(&hint->devices, i);
+		ast_free(device);
+	}
+	AST_VECTOR_FREE(&hint->devices);
 	ast_free(hint->last_presence_subtype);
 	ast_free(hint->last_presence_message);
 }
@@ -5787,6 +5852,7 @@ static int ast_add_hint(struct ast_exten *e)
 	if (!hint_new) {
 		return -1;
 	}
+	AST_VECTOR_INIT(&hint_new->devices, 8);
 
 	/* Initialize new hint. */
 	hint_new->callbacks = ao2_container_alloc(1, NULL, hint_id_cmp);
@@ -12526,14 +12592,17 @@ static int statecbs_cmp(void *obj, void *arg, int flags)
 static void pbx_shutdown(void)
 {
 	if (hints) {
+		ao2_container_unregister("hints");
 		ao2_ref(hints, -1);
 		hints = NULL;
 	}
 	if (hintdevices) {
+		ao2_container_unregister("hintdevices");
 		ao2_ref(hintdevices, -1);
 		hintdevices = NULL;
 	}
 	if (statecbs) {
+		ao2_container_unregister("statecbs");
 		ao2_ref(statecbs, -1);
 		statecbs = NULL;
 	}
@@ -12543,11 +12612,53 @@ static void pbx_shutdown(void)
 	pbx_builtin_clear_globals();
 }
 
+static void print_hints_key(void *v_obj, void *where, ao2_prnt_fn *prnt)
+{
+	struct ast_hint *hint = v_obj;
+
+	if (!hint) {
+		return;
+	}
+	prnt(where, "%s@%s", ast_get_extension_name(hint->exten),
+		ast_get_context_name(ast_get_extension_context(hint->exten)));
+}
+
+static void print_hintdevices_key(void *v_obj, void *where, ao2_prnt_fn *prnt)
+{
+	struct ast_hintdevice *hintdevice = v_obj;
+
+	if (!hintdevice) {
+		return;
+	}
+	prnt(where, "%s => %s@%s", hintdevice->hintdevice,
+		ast_get_extension_name(hintdevice->hint->exten),
+		ast_get_context_name(ast_get_extension_context(hintdevice->hint->exten)));
+}
+
+static void print_statecbs_key(void *v_obj, void *where, ao2_prnt_fn *prnt)
+{
+	struct ast_state_cb *state_cb = v_obj;
+
+	if (!state_cb) {
+		return;
+	}
+	prnt(where, "%d", state_cb->id);
+}
+
 int ast_pbx_init(void)
 {
 	hints = ao2_container_alloc(HASH_EXTENHINT_SIZE, hint_hash, hint_cmp);
+	if (hints) {
+		ao2_container_register("hints", hints, print_hints_key);
+	}
 	hintdevices = ao2_container_alloc(HASH_EXTENHINT_SIZE, hintdevice_hash_cb, hintdevice_cmp_multiple);
+	if (hintdevices) {
+		ao2_container_register("hintdevices", hintdevices, print_hintdevices_key);
+	}
 	statecbs = ao2_container_alloc(1, NULL, statecbs_cmp);
+	if (statecbs) {
+		ao2_container_register("statecbs", statecbs, print_statecbs_key);
+	}
 
 	ast_register_cleanup(pbx_shutdown);