[Asterisk-code-review] main/pbx: Improve performance of dialplan reloads with large... (asterisk[11])
Matt Jordan
asteriskteam at digium.com
Thu Apr 30 11:31:38 CDT 2015
Matt Jordan has uploaded a new change for review.
https://gerrit.asterisk.org/314
Change subject: main/pbx: Improve performance of dialplan reloads with large numbers of hints
......................................................................
main/pbx: Improve performance of dialplan reloads with large numbers 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.
ASTERISK-25040 #close
Reported by: Matt Jordan
Change-Id: Iedfc97a69d21070c50fca42275d7b3e714e59360
(cherry picked from commit fad4d7bfb83ca6167934c626370e7173a25fea08)
---
M main/pbx.c
1 file changed, 87 insertions(+), 23 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/14/314/1
diff --git a/main/pbx.c b/main/pbx.c
index 3f62a4d..3f0cf64 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -72,6 +72,8 @@
#include "asterisk/taskprocessor.h"
#include "asterisk/xmldoc.h"
#include "asterisk/astobj2.h"
+#include "asterisk/dial.h"
+#include "asterisk/vector.h"
/*!
* \note I M P O R T A N T :
@@ -988,8 +990,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);
@@ -1019,15 +1022,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_POINTER | OBJ_KEY)) {
+ case OBJ_KEY:
+ key = obj;
+ break;
+ case OBJ_POINTER:
+ 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
@@ -1036,29 +1052,55 @@
*/
static int hintdevice_cmp_multiple(void *obj, void *arg, int flags)
{
- struct ast_hintdevice *ext = obj, *ext2 = arg;
+ struct ast_hintdevice *left = arg;
+ struct ast_hintdevice *right = arg;
+ const char *right_key = arg;
+ int cmp;
- return !strcasecmp(ext->hintdevice, ext2->hintdevice) ? CMP_MATCH : 0;
-}
-
-/*
- * \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)
-{
- struct ast_hintdevice *device = deviceobj;
- struct ast_hint *hint = arg;
-
- return (device->hint == hint) ? CMP_MATCH : 0;
+ switch (flags & (OBJ_POINTER | OBJ_KEY)) {
+ case OBJ_POINTER:
+ right_key = right->hintdevice;
+ /* Fall through */
+ case OBJ_KEY:
+ cmp = strcmp(left->hintdevice, right_key);
+ break;
+ default:
+ /* Sort can only work on something with a full or partial key. */
+ ast_assert(0);
+ cmp = 0;
+ break;
+ }
+ return cmp ? 0 : CMP_MATCH;
}
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");
+ int i;
+
+ for (i = 0; i < AST_VECTOR_SIZE(&hint->devices); i++) {
+ struct ao2_iterator *dev_iter;
+ char *device = AST_VECTOR_GET(&hint->devices, i);
+
+ if (!device) {
+ continue;
+ }
+
+ dev_iter = ao2_t_callback(hintdevices, OBJ_KEY | OBJ_MULTIPLE,
+ hintdevice_cmp_multiple, device, "find devices in container");
+ if (dev_iter) {
+ struct ast_hintdevice *hintdevice;
+ for (; (hintdevice = ao2_iterator_next(dev_iter)); ao2_ref(hintdevice, -1)) {
+ /* Only remove those hintdevices for our hint */
+ if (hintdevice->hint == hint) {
+ ao2_unlink(hintdevices, hintdevice);
+ }
+ }
+ }
+ AST_VECTOR_REMOVE_UNORDERED(&hint->devices, i);
+ ast_free(device);
+ AST_VECTOR_INSERT(&hint->devices, i, NULL);
+ }
+
return 0;
}
@@ -1101,11 +1143,24 @@
ast_str_set(&str, 0, "%s", devicelist);
parse = parse_hint_device(str);
- while ((cur = strsep(&parse, "&"))) {
+ /* 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);
@@ -1113,6 +1168,7 @@
device->hint = hint;
ao2_t_link(hintdevices, device, "Linking device into hintdevice container.");
ao2_t_ref(device, -1, "hintdevice is linked so we can unref");
+ AST_VECTOR_APPEND(&hint->devices, device_name);
}
return 0;
@@ -5785,6 +5841,7 @@
static void destroy_hint(void *obj)
{
struct ast_hint *hint = obj;
+ int i;
if (hint->callbacks) {
struct ast_state_cb *state_cb;
@@ -5814,6 +5871,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);
}
@@ -5875,6 +5938,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);
--
To view, visit https://gerrit.asterisk.org/314
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iedfc97a69d21070c50fca42275d7b3e714e59360
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
More information about the asterisk-code-review
mailing list