[Asterisk-code-review] cdr.c: Defer getting ao2 global obj ref() until needed. (asterisk[15])

Richard Mudgett asteriskteam at digium.com
Thu Oct 12 12:33:23 CDT 2017


Richard Mudgett has uploaded this change for review. ( https://gerrit.asterisk.org/6778


Change subject: cdr.c: Defer getting ao2_global_obj_ref() until needed.
......................................................................

cdr.c: Defer getting ao2_global_obj_ref() until needed.

The CDR performance gets worse the further it gets behind in processing
stasis messages.  One of the reasons is we were getting the global config
even if we didn't need it.

* Most uses of the global config were only needed on off nominal code
paths so it makes sense to not get it until absolutely needed.

ASTERISK-27335

Change-Id: I00c63b7ec233e5bfffd5d976f05568613d3c2365
---
M main/cdr.c
1 file changed, 46 insertions(+), 30 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/78/6778/1

diff --git a/main/cdr.c b/main/cdr.c
index 6a3365e..6d2c96c 100644
--- a/main/cdr.c
+++ b/main/cdr.c
@@ -950,6 +950,27 @@
 }
 
 /*!
+ * \internal
+ * \brief Determine if CDR_END_BEFORE_H_EXTEN is configured.
+ *
+ * \retval 0 if CDR_END_BEFORE_H_EXTEN not configured.
+ * \retval non-zero if CDR_END_BEFORE_H_EXTEN is configured.
+ *
+ * \return Nothing
+ */
+static int is_end_before_h_exten_set(void)
+{
+	struct module_config *mod_cfg;
+	int end_before_h_exten;
+
+	mod_cfg = ao2_global_obj_ref(module_configs);
+	end_before_h_exten = !mod_cfg
+		|| ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN);
+	ao2_cleanup(mod_cfg);
+	return end_before_h_exten;
+}
+
+/*!
  * \brief Return whether or not a channel has changed its state in the dialplan, subject
  * to endbeforehexten logic
  *
@@ -962,12 +983,9 @@
 static int snapshot_cep_changed(struct ast_channel_snapshot *old_snapshot,
 	struct ast_channel_snapshot *new_snapshot)
 {
-	RAII_VAR(struct module_config *, mod_cfg,
-		ao2_global_obj_ref(module_configs), ao2_cleanup);
-
 	/* If we ignore hangup logic, don't indicate that we're executing anything new */
-	if (ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN)
-		&& ast_test_flag(&new_snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)) {
+	if (ast_test_flag(&new_snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)
+		&& is_end_before_h_exten_set()) {
 		return 0;
 	}
 
@@ -975,10 +993,11 @@
 	 * will attempt to clear the application and restore the dummy originate application
 	 * of "AppDialX". Ignore application changes to AppDialX as a result.
 	 */
-	if (strcmp(new_snapshot->appl, old_snapshot->appl) && strncasecmp(new_snapshot->appl, "appdial", 7)
+	if (strcmp(new_snapshot->appl, old_snapshot->appl)
+		&& strncasecmp(new_snapshot->appl, "appdial", 7)
 		&& (strcmp(new_snapshot->context, old_snapshot->context)
-		|| strcmp(new_snapshot->exten, old_snapshot->exten)
-		|| new_snapshot->priority != old_snapshot->priority)) {
+			|| strcmp(new_snapshot->exten, old_snapshot->exten)
+			|| new_snapshot->priority != old_snapshot->priority)) {
 		return 1;
 	}
 
@@ -1219,8 +1238,7 @@
  */
 static void cdr_object_set_disposition(struct cdr_object *cdr, int hangupcause)
 {
-	RAII_VAR(struct module_config *, mod_cfg,
-			ao2_global_obj_ref(module_configs), ao2_cleanup);
+	struct module_config *mod_cfg;
 
 	/* Change the disposition based on the hang up cause */
 	switch (hangupcause) {
@@ -1228,11 +1246,14 @@
 		cdr->disposition = AST_CDR_BUSY;
 		break;
 	case AST_CAUSE_CONGESTION:
-		if (!ast_test_flag(&mod_cfg->general->settings, CDR_CONGESTION)) {
+		mod_cfg = ao2_global_obj_ref(module_configs);
+		if (!mod_cfg
+			|| !ast_test_flag(&mod_cfg->general->settings, CDR_CONGESTION)) {
 			cdr->disposition = AST_CDR_FAILED;
 		} else {
 			cdr->disposition = AST_CDR_CONGESTION;
 		}
+		ao2_cleanup(mod_cfg);
 		break;
 	case AST_CAUSE_NO_ROUTE_DESTINATION:
 	case AST_CAUSE_UNREGISTERED:
@@ -1295,10 +1316,8 @@
  */
 static void cdr_object_check_party_a_hangup(struct cdr_object *cdr)
 {
-	RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);
-
-	if (ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN)
-		&& ast_test_flag(&cdr->party_a.snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)) {
+	if (ast_test_flag(&cdr->party_a.snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)
+		&& is_end_before_h_exten_set()) {
 		cdr_object_finalize(cdr);
 	}
 
@@ -1361,13 +1380,11 @@
 
 static int base_process_party_a(struct cdr_object *cdr, struct ast_channel_snapshot *snapshot)
 {
-	RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);
-
 	ast_assert(strcasecmp(snapshot->name, cdr->party_a.snapshot->name) == 0);
 
 	/* Finalize the CDR if we're in hangup logic and we're set to do so */
 	if (ast_test_flag(&snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)
-		&& ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN)) {
+		&& is_end_before_h_exten_set()) {
 		cdr_object_finalize(cdr);
 		return 0;
 	}
@@ -1651,9 +1668,6 @@
  */
 static enum ast_cdr_disposition dial_status_to_disposition(const char *dial_status)
 {
-	RAII_VAR(struct module_config *, mod_cfg,
-		ao2_global_obj_ref(module_configs), ao2_cleanup);
-
 	if (!strcmp(dial_status, "ANSWER")) {
 		return AST_CDR_ANSWERED;
 	} else if (!strcmp(dial_status, "BUSY")) {
@@ -1661,7 +1675,15 @@
 	} else if (!strcmp(dial_status, "CANCEL") || !strcmp(dial_status, "NOANSWER")) {
 		return AST_CDR_NOANSWER;
 	} else if (!strcmp(dial_status, "CONGESTION")) {
-		if (!ast_test_flag(&mod_cfg->general->settings, CDR_CONGESTION)) {
+		struct module_config *mod_cfg;
+		int is_congestion;
+
+		mod_cfg = ao2_global_obj_ref(module_configs);
+		is_congestion = mod_cfg
+			&& ast_test_flag(&mod_cfg->general->settings, CDR_CONGESTION);
+		ao2_cleanup(mod_cfg);
+
+		if (!is_congestion) {
 			return AST_CDR_FAILED;
 		} else {
 			return AST_CDR_CONGESTION;
@@ -1871,11 +1893,8 @@
 
 static int finalized_state_process_party_a(struct cdr_object *cdr, struct ast_channel_snapshot *snapshot)
 {
-	RAII_VAR(struct module_config *, mod_cfg,
-		ao2_global_obj_ref(module_configs), ao2_cleanup);
-
 	if (ast_test_flag(&snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)
-			&& ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN)) {
+		&& is_end_before_h_exten_set()) {
 		return 0;
 	}
 
@@ -2054,13 +2073,10 @@
 static int check_new_cdr_needed(struct ast_channel_snapshot *old_snapshot,
 		struct ast_channel_snapshot *new_snapshot)
 {
-	RAII_VAR(struct module_config *, mod_cfg,
-			ao2_global_obj_ref(module_configs), ao2_cleanup);
-
 	/* If we're dead, we don't need a new CDR */
 	if (!new_snapshot
 		|| (ast_test_flag(&new_snapshot->softhangup_flags, AST_SOFTHANGUP_HANGUP_EXEC)
-			&& ast_test_flag(&mod_cfg->general->settings, CDR_END_BEFORE_H_EXTEN))) {
+			&& is_end_before_h_exten_set())) {
 		return 0;
 	}
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: I00c63b7ec233e5bfffd5d976f05568613d3c2365
Gerrit-Change-Number: 6778
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171012/32557be3/attachment-0001.html>


More information about the asterisk-code-review mailing list