From 61b23a50c735569f212a38641cb61c7a8dc93ff4 Mon Sep 17 00:00:00 2001 From: Liviu Chircu Date: Fri, 27 Sep 2019 11:14:23 +0300 Subject: [PATCH] dialog: Various small code/performance improvements * remove redundant "is_replicated" function parameter * speed up profile hash computation (size is always 2^N, see new_dlg_profile() -> we can do bit AND) * shorten cookie-cutter code with add_last() and str_cpy() * fix over-complicated if condition * do not leak PKG memory on some error case * remove useless comment (cherry picked from commit 20a962a23879d66007bd5fdcbf1e314c5181d1cc) --- modules/dialog/dlg_handlers.c | 6 +++--- modules/dialog/dlg_hash.c | 10 +++++----- modules/dialog/dlg_hash.h | 2 +- modules/dialog/dlg_profile.c | 27 ++++++++++----------------- modules/dialog/dlg_profile.h | 4 ++-- modules/dialog/dlg_repl_profile.h | 6 ++---- modules/dialog/dlg_replication.c | 4 ++-- modules/dialog/dlg_req_within.c | 2 +- 8 files changed, 26 insertions(+), 35 deletions(-) diff --git a/modules/dialog/dlg_handlers.c b/modules/dialog/dlg_handlers.c index 511bbcc5d47..1b707662528 100644 --- a/modules/dialog/dlg_handlers.c +++ b/modules/dialog/dlg_handlers.c @@ -599,7 +599,7 @@ static void dlg_onreply(struct cell* t, int type, struct tmcb_params *param) LM_DBG("dialog %p failed (negative reply)\n", dlg); /*destroy profile linkers */ - destroy_linkers(dlg, 0); + destroy_linkers(dlg); remove_dlg_prof_table(dlg, 0); /* dialog setup not completed (3456XX) */ @@ -1702,7 +1702,7 @@ void dlg_onroute(struct sip_msg* req, str *route_params, void *param) old_state!=DLG_STATE_DELETED) { /*destroy profile linkers */ - destroy_linkers(dlg, 0); + destroy_linkers(dlg); remove_dlg_prof_table(dlg,0); if (!dlg->terminate_reason.s) { @@ -2088,7 +2088,7 @@ void dlg_ontimeout(struct dlg_tl *tl) ZSW(dlg->legs[callee_idx(dlg)].tag.s)); /*destroy profile linkers */ - destroy_linkers(dlg, 0); + destroy_linkers(dlg); remove_dlg_prof_table(dlg,0); /* dialog timeout */ diff --git a/modules/dialog/dlg_hash.c b/modules/dialog/dlg_hash.c index 1ffda215ac3..442f28de116 100644 --- a/modules/dialog/dlg_hash.c +++ b/modules/dialog/dlg_hash.c @@ -195,7 +195,7 @@ static inline void free_dlg_dlg(struct dlg_cell *dlg) context_destroy(CONTEXT_DIALOG, context_of(dlg)); if (dlg->profile_links) { - destroy_linkers_unsafe(dlg, 0); + destroy_linkers_unsafe(dlg); remove_dlg_prof_table(dlg, 0); } @@ -914,7 +914,7 @@ struct dlg_cell* get_dlg_by_callid( str *callid, int active_only) } -void link_dlg(struct dlg_cell *dlg, int n) +void link_dlg(struct dlg_cell *dlg, int extra_refs) { struct dlg_entry *d_entry; @@ -924,11 +924,11 @@ void link_dlg(struct dlg_cell *dlg, int n) link_dlg_unsafe(d_entry, dlg); - DBG_REF(dlg, n); - dlg->ref += n; + DBG_REF(dlg, extra_refs); + dlg->ref += extra_refs; LM_DBG("ref dlg %p with %d -> %d in h_entry %p - %d \n", - dlg, n + 1, dlg->ref, d_entry, dlg->h_entry); + dlg, extra_refs + 1, dlg->ref, d_entry, dlg->h_entry); dlg_unlock( d_table, d_entry); } diff --git a/modules/dialog/dlg_hash.h b/modules/dialog/dlg_hash.h index b63f9529ee8..1df0150aa76 100644 --- a/modules/dialog/dlg_hash.h +++ b/modules/dialog/dlg_hash.h @@ -399,7 +399,7 @@ struct dlg_cell* get_dlg_by_val(str *attr, str *val); struct dlg_cell* get_dlg_by_callid( str *callid, int active_only); -void link_dlg(struct dlg_cell *dlg, int n); +void link_dlg(struct dlg_cell *dlg, int extra_refs); #define _link_dlg_unsafe(d_entry, dlg) \ do { \ diff --git a/modules/dialog/dlg_profile.c b/modules/dialog/dlg_profile.c index 09e9ed19212..64efc719bd5 100644 --- a/modules/dialog/dlg_profile.c +++ b/modules/dialog/dlg_profile.c @@ -439,7 +439,6 @@ static struct dlg_profile_table* new_dlg_profile( str *name, unsigned int size, unsigned int has_value, unsigned repl_type) { struct dlg_profile_table *profile; - struct dlg_profile_table *ptmp; unsigned int len; unsigned int i; @@ -524,17 +523,10 @@ static struct dlg_profile_table* new_dlg_profile( str *name, unsigned int size, size*sizeof(struct prof_local_count*); } - /* copy the name of the profile */ - memcpy( profile->name.s, name->s, name->len ); - profile->name.len = name->len; - profile->name.s[profile->name.len] = 0; + str_cpy(&profile->name, name); + profile->name.s[profile->name.len] = '\0'; - /* link profile */ - for( ptmp=profiles ; ptmp && ptmp->next; ptmp=ptmp->next ); - if (ptmp==NULL) - profiles = profile; - else - ptmp->next = profile; + add_last(profile, profiles); return profile; } @@ -619,7 +611,7 @@ static int init_tmp_linkers(struct dlg_cell *dlg) return 0; } -void destroy_linkers_unsafe(struct dlg_cell *dlg, char is_replicated) +void destroy_linkers_unsafe(struct dlg_cell *dlg) { struct dlg_profile_link *l, *linker = dlg->profile_links; @@ -640,11 +632,11 @@ void destroy_linkers_unsafe(struct dlg_cell *dlg, char is_replicated) dlg->profile_links = NULL; } -void destroy_linkers(struct dlg_cell *dlg, char is_replicated) +void destroy_linkers(struct dlg_cell *dlg) { dlg_lock_dlg(dlg); - destroy_linkers_unsafe(dlg, is_replicated); + destroy_linkers_unsafe(dlg); dlg_unlock_dlg(dlg); } @@ -759,7 +751,7 @@ inline static unsigned int calc_hash_profile( str *value, struct dlg_cell *dlg, return core_hash( value, NULL, profile->size); } else { /* do hash over dialog pointer */ - return ((unsigned long)dlg) % profile->size ; + return ((unsigned long)dlg) & (profile->size - 1); } } @@ -985,7 +977,7 @@ int is_dlg_in_profile(struct dlg_cell *dlg, struct dlg_profile_table *profile, dlg_lock( d_table, d_entry); for( linker=dlg->profile_links ; linker ; linker=linker->next) { if (linker->profile==profile) { - if (profile->has_value==0 || (profile->has_value==1 && !value)) { + if (!profile->has_value || !value) { dlg_unlock( d_table, d_entry); return 1; } else if (value->len==linker->value.len && @@ -1519,8 +1511,9 @@ static mi_response_t *mi_profile_terminate(const mi_params_t *params, str *value )) { delete_entry = pkg_malloc(sizeof(struct dialog_list)); if (!delete_entry) { - LM_CRIT("no more pkg memory\n"); lock_set_release(d_table->locks,d_entry->lock_idx); + pkg_free_all(deleted); + LM_CRIT("no more pkg memory\n"); return init_mi_error(400, MI_SSTR("Internal error")); } diff --git a/modules/dialog/dlg_profile.h b/modules/dialog/dlg_profile.h index 4e8fa437213..117ddd88e79 100644 --- a/modules/dialog/dlg_profile.h +++ b/modules/dialog/dlg_profile.h @@ -113,8 +113,8 @@ void destroy_dlg_profiles(); struct dlg_profile_table* search_dlg_profile(str *name); struct dlg_profile_table *get_dlg_profile(str *name); -void destroy_linkers(struct dlg_cell *dlg, char is_replicated); -void destroy_linkers_unsafe(struct dlg_cell *dlg, char is_replicated); +void destroy_linkers(struct dlg_cell *dlg); +void destroy_linkers_unsafe(struct dlg_cell *dlg); void remove_dlg_prof_table(struct dlg_cell *dlg, char is_replicated); int set_dlg_profile(struct dlg_cell *dlg, str *value, diff --git a/modules/dialog/dlg_repl_profile.h b/modules/dialog/dlg_repl_profile.h index 217bd01f3d1..29b5ce0e5fc 100644 --- a/modules/dialog/dlg_repl_profile.h +++ b/modules/dialog/dlg_repl_profile.h @@ -98,9 +98,7 @@ static inline prof_rcv_count_t *repl_prof_allocate(void) rp = shm_malloc(sizeof(prof_rcv_count_t)); if (!rp) { - /* if there is no more shm memory, there's not much that you can do - * anyway */ - LM_WARN("no more shm mem\n"); + LM_ERR("no more shm mem\n"); return NULL; } @@ -195,7 +193,7 @@ static inline void prof_val_local_inc(void **pv_info, str *shtag, int is_repl) cnt->n++; } else { - (*pv_info) = (void*)((long)(*pv_info) + 1); + *pv_info = (void*)((long)(*pv_info) + 1); } } diff --git a/modules/dialog/dlg_replication.c b/modules/dialog/dlg_replication.c index d5cd86b39cb..17123d456df 100644 --- a/modules/dialog/dlg_replication.c +++ b/modules/dialog/dlg_replication.c @@ -442,7 +442,7 @@ int dlg_replicated_delete(bin_packet_t *packet) return 0; } - destroy_linkers(dlg, 1); + destroy_linkers(dlg); remove_dlg_prof_table(dlg, 1); /* simulate BYE received from caller */ @@ -919,7 +919,7 @@ void rcv_cluster_event(enum clusterer_event ev, int node_id) unref++; /* the extra added ref */ dlg_lock(d_table, &d_table->entries[i]); - destroy_linkers_unsafe(dlg, 0); + destroy_linkers_unsafe(dlg); dlg_unlock(d_table, &d_table->entries[i]); diff --git a/modules/dialog/dlg_req_within.c b/modules/dialog/dlg_req_within.c index e71127c1abd..a39738ef6ec 100644 --- a/modules/dialog/dlg_req_within.c +++ b/modules/dialog/dlg_req_within.c @@ -221,7 +221,7 @@ static void dual_bye_event(struct dlg_cell* dlg, struct sip_msg *req, dlg->h_entry, dlg->h_id); /*destroy linkers */ - destroy_linkers(dlg, 0); + destroy_linkers(dlg); remove_dlg_prof_table(dlg,0); /* remove from timer */