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

Jenkins2 asteriskteam at digium.com
Fri Oct 13 18:13:39 CDT 2017


Jenkins2 has submitted this change and it was merged. ( 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, 40 insertions(+), 41 deletions(-)

Approvals:
  Corey Farrell: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/main/cdr.c b/main/cdr.c
index 6a3365e..dc5c947 100644
--- a/main/cdr.c
+++ b/main/cdr.c
@@ -950,6 +950,28 @@
 }
 
 /*!
+ * \internal
+ * \brief Determine if CDR flag is configured.
+ *
+ * \param cdr_flag The configured CDR flag to check.
+ *
+ * \retval 0 if the CDR flag is not configured.
+ * \retval non-zero if the CDR flag is configured.
+ *
+ * \return Nothing
+ */
+static int is_cdr_flag_set(unsigned int cdr_flag)
+{
+	struct module_config *mod_cfg;
+	int flag_set;
+
+	mod_cfg = ao2_global_obj_ref(module_configs);
+	flag_set = mod_cfg && ast_test_flag(&mod_cfg->general->settings, cdr_flag);
+	ao2_cleanup(mod_cfg);
+	return flag_set;
+}
+
+/*!
  * \brief Return whether or not a channel has changed its state in the dialplan, subject
  * to endbeforehexten logic
  *
@@ -962,12 +984,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_cdr_flag_set(CDR_END_BEFORE_H_EXTEN)) {
 		return 0;
 	}
 
@@ -975,10 +994,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;
 	}
 
@@ -1050,15 +1070,15 @@
  */
 static long cdr_object_get_billsec(struct cdr_object *cdr)
 {
-	RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);
 	long int ms;
 
 	if (ast_tvzero(cdr->answer)) {
 		return 0;
 	}
+
 	ms = ast_tvdiff_ms(ast_tvzero(cdr->end) ? ast_tvnow() : cdr->end, cdr->answer);
-	if (ast_test_flag(&mod_cfg->general->settings, CDR_INITIATED_SECONDS)
-		&& (ms % 1000 >= 500)) {
+	if (ms % 1000 >= 500
+		&& is_cdr_flag_set(CDR_INITIATED_SECONDS)) {
 		ms = (ms / 1000) + 1;
 	} else {
 		ms = ms / 1000;
@@ -1219,16 +1239,13 @@
  */
 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);
-
 	/* Change the disposition based on the hang up cause */
 	switch (hangupcause) {
 	case AST_CAUSE_BUSY:
 		cdr->disposition = AST_CDR_BUSY;
 		break;
 	case AST_CAUSE_CONGESTION:
-		if (!ast_test_flag(&mod_cfg->general->settings, CDR_CONGESTION)) {
+		if (!is_cdr_flag_set(CDR_CONGESTION)) {
 			cdr->disposition = AST_CDR_FAILED;
 		} else {
 			cdr->disposition = AST_CDR_CONGESTION;
@@ -1295,10 +1312,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_cdr_flag_set(CDR_END_BEFORE_H_EXTEN)) {
 		cdr_object_finalize(cdr);
 	}
 
@@ -1361,13 +1376,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_cdr_flag_set(CDR_END_BEFORE_H_EXTEN)) {
 		cdr_object_finalize(cdr);
 		return 0;
 	}
@@ -1651,9 +1664,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 +1671,7 @@
 	} 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)) {
+		if (!is_cdr_flag_set(CDR_CONGESTION)) {
 			return AST_CDR_FAILED;
 		} else {
 			return AST_CDR_CONGESTION;
@@ -1871,11 +1881,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_cdr_flag_set(CDR_END_BEFORE_H_EXTEN)) {
 		return 0;
 	}
 
@@ -2054,13 +2061,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_cdr_flag_set(CDR_END_BEFORE_H_EXTEN))) {
 		return 0;
 	}
 
@@ -2660,8 +2664,7 @@
 
 int ast_cdr_is_enabled(void)
 {
-	RAII_VAR(struct module_config *, mod_cfg, ao2_global_obj_ref(module_configs), ao2_cleanup);
-	return ast_test_flag(&mod_cfg->general->settings, CDR_ENABLED);
+	return is_cdr_flag_set(CDR_ENABLED);
 }
 
 int ast_cdr_backend_suspend(const char *name)
@@ -3155,13 +3158,9 @@
 
 	cdr = cdr_object_get_by_name(channel_name);
 	if (!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_ENABLED)) {
+		if (is_cdr_flag_set(CDR_ENABLED)) {
 			ast_log(AST_LOG_ERROR, "Unable to find CDR for channel %s\n", channel_name);
 		}
-
 		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: merged
Gerrit-Change-Id: I00c63b7ec233e5bfffd5d976f05568613d3c2365
Gerrit-Change-Number: 6778
Gerrit-PatchSet: 2
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171013/e3aa9689/attachment.html>


More information about the asterisk-code-review mailing list