[Asterisk-code-review] Manager: Short circuit AMI message processing. (asterisk[master])

Joshua Colp asteriskteam at digium.com
Tue Apr 26 04:57:36 CDT 2016


Joshua Colp has submitted this change and it was merged.

Change subject: Manager: Short circuit AMI message processing.
......................................................................


Manager: Short circuit AMI message processing.

Improve AMI message processing performance if there are no consumers
listening for the messages.  We now skip creating the AMI event message
text strings.

Change-Id: I7b22fc5ec4e500d00635c1a467aa8ea68a1bb2b3
---
M include/asterisk/stasis.h
M main/manager.c
M main/stasis_message.c
3 files changed, 153 insertions(+), 38 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved; Verified



diff --git a/include/asterisk/stasis.h b/include/asterisk/stasis.h
index 69b2d0f..14ab7d9 100644
--- a/include/asterisk/stasis.h
+++ b/include/asterisk/stasis.h
@@ -414,14 +414,14 @@
  * May return \c NULL, to indicate no representation. The returned object should
  * be ast_json_unref()'ed.
  *
- * \param message Message to convert to JSON string.
+ * \param msg Message to convert to JSON string.
  * \param sanitize Snapshot sanitization callback.
  *
  * \return Newly allocated string with JSON message.
  * \return \c NULL on error.
  * \return \c NULL if JSON format is not supported.
  */
-struct ast_json *stasis_message_to_json(struct stasis_message *message, struct stasis_message_sanitizer *sanitize);
+struct ast_json *stasis_message_to_json(struct stasis_message *msg, struct stasis_message_sanitizer *sanitize);
 
 /*!
  * \brief Build the AMI representation of the message.
@@ -429,12 +429,21 @@
  * May return \c NULL, to indicate no representation. The returned object should
  * be ao2_cleanup()'ed.
  *
- * \param message Message to convert to AMI.
+ * \param msg Message to convert to AMI.
  * \return \c NULL on error.
  * \return \c NULL if AMI format is not supported.
  */
-struct ast_manager_event_blob *stasis_message_to_ami(
-	struct stasis_message *message);
+struct ast_manager_event_blob *stasis_message_to_ami(struct stasis_message *msg);
+
+/*!
+ * \brief Determine if the given message can be converted to AMI.
+ *
+ * \param msg Message to see if can be converted to AMI.
+ *
+ * \retval 0 Cannot be converted
+ * \retval non-zero Can be converted
+ */
+int stasis_message_can_be_ami(struct stasis_message *msg);
 
 /*!
  * \brief Build the \ref AstGenericEvents representation of the message.
@@ -442,12 +451,11 @@
  * May return \c NULL, to indicate no representation. The returned object should
  * be disposed of via \ref ast_event_destroy.
  *
- * \param message Message to convert to AMI.
+ * \param msg Message to convert to AMI.
  * \return \c NULL on error.
  * \return \c NULL if AMI format is not supported.
  */
-struct ast_event *stasis_message_to_event(
-	struct stasis_message *message);
+struct ast_event *stasis_message_to_event(struct stasis_message *msg);
 
 /*!
  * \brief A topic to which messages may be posted, and subscribers, well, subscribe
diff --git a/main/manager.c b/main/manager.c
index 0cef582..d2fdc40 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -1549,6 +1549,17 @@
 /*! \brief A container of event documentation nodes */
 static AO2_GLOBAL_OBJ_STATIC(event_docs);
 
+static int __attribute__((format(printf, 9, 0))) __manager_event_sessions(
+	struct ao2_container *sessions,
+	int category,
+	const char *event,
+	int chancount,
+	struct ast_channel **chans,
+	const char *file,
+	int line,
+	const char *func,
+	const char *fmt,
+	...);
 static enum add_filter_result manager_add_filter(const char *filter_pattern, struct ao2_container *whitefilters, struct ao2_container *blackfilters);
 
 static int match_filter(struct mansession *s, char *eventdata);
@@ -1687,38 +1698,75 @@
 	return res;
 }
 
+#define manager_event_sessions(sessions, category, event, contents , ...)	\
+	__manager_event_sessions(sessions, category, event, 0, NULL, __FILE__, __LINE__, __PRETTY_FUNCTION__, contents , ## __VA_ARGS__)
+
+#define any_manager_listeners(sessions)	\
+	((sessions && ao2_container_count(sessions)) || !AST_RWLIST_EMPTY(&manager_hooks))
+
 static void manager_default_msg_cb(void *data, struct stasis_subscription *sub,
 				    struct stasis_message *message)
 {
+	struct ao2_container *sessions;
 	struct ast_manager_event_blob *ev;
 
-	ev = stasis_message_to_ami(message);
-	if (!ev) {
+	if (!stasis_message_can_be_ami(message)) {
 		/* Not an AMI message; disregard */
 		return;
 	}
 
-	manager_event(ev->event_flags, ev->manager_event, "%s",
-		ev->extra_fields);
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (!any_manager_listeners(sessions)) {
+		/* Nobody is listening */
+		ao2_cleanup(sessions);
+		return;
+	}
+
+	ev = stasis_message_to_ami(message);
+	if (!ev) {
+		/* Conversion failure */
+		ao2_cleanup(sessions);
+		return;
+	}
+
+	manager_event_sessions(sessions, ev->event_flags, ev->manager_event,
+		"%s", ev->extra_fields);
 	ao2_ref(ev, -1);
+	ao2_cleanup(sessions);
 }
 
 static void manager_generic_msg_cb(void *data, struct stasis_subscription *sub,
 				    struct stasis_message *message)
 {
-	struct ast_json_payload *payload = stasis_message_data(message);
-	int class_type = ast_json_integer_get(ast_json_object_get(payload->json, "class_type"));
-	const char *type = ast_json_string_get(ast_json_object_get(payload->json, "type"));
-	struct ast_json *event = ast_json_object_get(payload->json, "event");
+	struct ast_json_payload *payload;
+	int class_type;
+	const char *type;
+	struct ast_json *event;
 	struct ast_str *event_buffer;
+	struct ao2_container *sessions;
+
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (!any_manager_listeners(sessions)) {
+		/* Nobody is listening */
+		ao2_cleanup(sessions);
+		return;
+	}
+
+	payload = stasis_message_data(message);
+	class_type = ast_json_integer_get(ast_json_object_get(payload->json, "class_type"));
+	type = ast_json_string_get(ast_json_object_get(payload->json, "type"));
+	event = ast_json_object_get(payload->json, "event");
 
 	event_buffer = ast_manager_str_from_json_object(event, NULL);
 	if (!event_buffer) {
 		ast_log(AST_LOG_WARNING, "Error while creating payload for event %s\n", type);
+		ao2_cleanup(sessions);
 		return;
 	}
-	manager_event(class_type, type, "%s", ast_str_buffer(event_buffer));
+	manager_event_sessions(sessions, class_type, type,
+		"%s", ast_str_buffer(event_buffer));
 	ast_free(event_buffer);
+	ao2_cleanup(sessions);
 }
 
 void ast_manager_publish_event(const char *type, int class_type, struct ast_json *obj)
@@ -6645,21 +6693,23 @@
 AST_THREADSTORAGE(manager_event_buf);
 #define MANAGER_EVENT_BUF_INITSIZE   256
 
-int __ast_manager_event_multichan(int category, const char *event, int chancount,
-	struct ast_channel **chans, const char *file, int line, const char *func,
-	const char *fmt, ...)
+static int __attribute__((format(printf, 9, 0))) __manager_event_sessions_va(
+	struct ao2_container *sessions,
+	int category,
+	const char *event,
+	int chancount,
+	struct ast_channel **chans,
+	const char *file,
+	int line,
+	const char *func,
+	const char *fmt,
+	va_list ap)
 {
-	RAII_VAR(struct ao2_container *, sessions, ao2_global_obj_ref(mgr_sessions), ao2_cleanup);
 	struct ast_str *auth = ast_str_alloca(MAX_AUTH_PERM_STRING);
 	const char *cat_str;
-	va_list ap;
 	struct timeval now;
 	struct ast_str *buf;
 	int i;
-
-	if (!(sessions && ao2_container_count(sessions)) && AST_RWLIST_EMPTY(&manager_hooks)) {
-		return 0;
-	}
 
 	buf = ast_str_thread_get(&manager_event_buf, MANAGER_EVENT_BUF_INITSIZE);
 	if (!buf) {
@@ -6696,9 +6746,7 @@
 			ast_config_AST_SYSTEM_NAME);
 	}
 
-	va_start(ap, fmt);
 	ast_str_append_va(&buf, 0, fmt, ap);
-	va_end(ap);
 	for (i = 0; i < chancount; i++) {
 		append_channel_vars(&buf, chans[i]);
 	}
@@ -6744,6 +6792,50 @@
 	return 0;
 }
 
+static int __attribute__((format(printf, 9, 0))) __manager_event_sessions(
+	struct ao2_container *sessions,
+	int category,
+	const char *event,
+	int chancount,
+	struct ast_channel **chans,
+	const char *file,
+	int line,
+	const char *func,
+	const char *fmt,
+	...)
+{
+	va_list ap;
+	int res;
+
+	va_start(ap, fmt);
+	res = __manager_event_sessions_va(sessions, category, event, chancount, chans,
+		file, line, func, fmt, ap);
+	va_end(ap);
+	return res;
+}
+
+int __ast_manager_event_multichan(int category, const char *event, int chancount,
+	struct ast_channel **chans, const char *file, int line, const char *func,
+	const char *fmt, ...)
+{
+	struct ao2_container *sessions = ao2_global_obj_ref(mgr_sessions);
+	va_list ap;
+	int res;
+
+	if (!any_manager_listeners(sessions)) {
+		/* Nobody is listening */
+		ao2_cleanup(sessions);
+		return 0;
+	}
+
+	va_start(ap, fmt);
+	res = __manager_event_sessions_va(sessions, category, event, chancount, chans,
+		file, line, func, fmt, ap);
+	va_end(ap);
+	ao2_cleanup(sessions);
+	return res;
+}
+
 /*! \brief
  * support functions to register/unregister AMI action handlers,
  */
diff --git a/main/stasis_message.c b/main/stasis_message.c
index 0e6ff92..37b9a2b 100644
--- a/main/stasis_message.c
+++ b/main/stasis_message.c
@@ -170,17 +170,17 @@
 	return &msg->timestamp;
 }
 
-#define INVOKE_VIRTUAL(fn, ...)				\
-	({						\
-		if (msg == NULL) {			\
-			return NULL;			\
-		}					\
-		ast_assert(msg->type != NULL);		\
+#define INVOKE_VIRTUAL(fn, ...)					\
+	({											\
+		if (!msg) {								\
+			return NULL;						\
+		}										\
+		ast_assert(msg->type != NULL);			\
 		ast_assert(msg->type->vtable != NULL);	\
-		if (msg->type->vtable->fn == NULL) {	\
-			return NULL;			\
-		}					\
-		msg->type->vtable->fn(__VA_ARGS__);	\
+		if (!msg->type->vtable->fn) {			\
+			return NULL;						\
+		}										\
+		msg->type->vtable->fn(__VA_ARGS__);		\
 	})
 
 struct ast_manager_event_blob *stasis_message_to_ami(struct stasis_message *msg)
@@ -199,3 +199,18 @@
 {
 	return INVOKE_VIRTUAL(to_event, msg);
 }
+
+#define HAS_VIRTUAL(fn, msg)					\
+	({											\
+		if (!msg) {								\
+			return 0;							\
+		}										\
+		ast_assert(msg->type != NULL);			\
+		ast_assert(msg->type->vtable != NULL);	\
+		!!msg->type->vtable->fn;				\
+	})
+
+int stasis_message_can_be_ami(struct stasis_message *msg)
+{
+	return HAS_VIRTUAL(to_ami, msg);
+}

-- 
To view, visit https://gerrit.asterisk.org/2593
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I7b22fc5ec4e500d00635c1a467aa8ea68a1bb2b3
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list