[Asterisk-code-review] main/pbx: Improve performance of dialplan reloads with a lar... (asterisk[master])
Matt Jordan
asteriskteam at digium.com
Sat May 2 10:16:54 CDT 2015
Matt Jordan has submitted this change and it was merged.
Change subject: main/pbx: Improve performance of dialplan reloads with a large number of hints
......................................................................
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
---
M main/pbx.c
1 file changed, 130 insertions(+), 19 deletions(-)
Approvals:
Mark Michelson: Looks good to me, but someone else must approve
Richard Mudgett: Looks good to me, but someone else must approve
Matt Jordan: Looks good to me, approved; Verified
diff --git a/main/pbx.c b/main/pbx.c
index fee4191..45909f5 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -72,6 +72,7 @@
#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 @@
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 @@
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_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 @@
/* 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 @@
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 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 @@
}
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 @@
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 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 @@
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);
--
To view, visit https://gerrit.asterisk.org/309
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iedfc97a69d21070c50fca42275d7b3e714e59360
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-code-review
mailing list