<p>Sean Bright has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/6989">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_pjsip: Improve CLI completion performance<br><br>Previously, this code would duplicate the target ao2 container and<br>iterate it for each of the objects within it. For a small number of<br>objects this was fine, but as the number of objects grows the<br>performance degrades quickly. For 4000 endpoints, the completion of:<br><br>  pjsip show endpoint <tab><br><br>Takes over 10 minutes on my development workstation.<br><br>This patch adds a cache of a given command'ss possible completions for<br>more performant iteration and retrieval. 4000 endpoints are now<br>completed in under 1 second on my development machine.<br><br>Change-Id: Ib6fdeb5283b7c1c66df22c0fa08e95524bf55011<br>---<br>M res/res_pjsip/pjsip_cli.c<br>1 file changed, 130 insertions(+), 71 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/89/6989/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/res/res_pjsip/pjsip_cli.c b/res/res_pjsip/pjsip_cli.c<br>index 56ec191..47516ab 100644<br>--- a/res/res_pjsip/pjsip_cli.c<br>+++ b/res/res_pjsip/pjsip_cli.c<br>@@ -82,31 +82,33 @@<br>      return 0;<br> }<br> <br>-static char *complete_show_sorcery_object(struct ao2_container *container,<br>-        struct ast_sip_cli_formatter_entry *formatter_entry,<br>- const char *word, int state)<br>+static char **completion_cache_create(struct ao2_container *container,<br>+         struct ast_sip_cli_formatter_entry *formatter_entry,<br>+         const char *word)<br> {<br>-       char *result = NULL;<br>- int wordlen = strlen(word);<br>-  int which = 0;<br>-<br>-    struct ao2_iterator i = ao2_iterator_init(container, 0);<br>+     size_t wordlen = strlen(word);<br>+       struct ao2_iterator i;<br>        void *object;<br>+        char **cache;<br>+        size_t count = ao2_container_count(container), idx = 0;<br> <br>+   /* Create a string array to cache our result set */<br>+  cache = ast_calloc(count + 1, sizeof(char *));<br>+       if (!cache) {<br>+                return NULL;<br>+ }<br>+<br>+ i = ao2_iterator_init(container, 0);<br>  while ((object = ao2_t_iterator_next(&i, "iterate thru endpoints table"))) {<br>            const char *id = formatter_entry->get_id(object);<br>-         if (!strncasecmp(word, id, wordlen)<br>-                  && ++which > state) {<br>-                     result = ast_strdup(id);<br>+             if (!strncasecmp(word, id, wordlen)) {<br>+                       cache[idx++] = ast_strdup(id);<br>                }<br>             ao2_t_ref(object, -1, "toss iterator endpoint ptr before break");<br>-          if (result) {<br>-                        break;<br>-               }<br>     }<br>     ao2_iterator_destroy(&i);<br> <br>-     return result;<br>+       return cache;<br> }<br> <br> static void dump_str_and_free(int fd, struct ast_str *buf)<br>@@ -115,16 +117,69 @@<br>      ast_free(buf);<br> }<br> <br>-char *ast_sip_cli_traverse_objects(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)<br>+AST_THREADSTORAGE_RAW(container_cache);<br>+<br>+static char *sip_cli_traverse_objects_generate(<br>+  struct ast_cli_entry *e,<br>+     struct ast_cli_args *a,<br>+      struct ast_sip_cli_formatter_entry *formatter_entry,<br>+ int is_container)<br>+{<br>+        char **cache, *res;<br>+<br>+       if (is_container<br>+        || a->argc > 4<br>+         || (a->argc == 4 && ast_strlen_zero(a->word))) {<br>+            return CLI_SUCCESS;<br>+  }<br>+<br>+ /* Should we get the container from TLS or create it? */<br>+     if (a->n == 0) {<br>+          struct ao2_container *container =<br>+                    formatter_entry->get_container("");<br>+<br>+          if (!container) {<br>+                    ast_cli(a->fd, "No container returned for object type %s.\n",<br>+                           formatter_entry->name);<br>+                   return CLI_FAILURE;<br>+          }<br>+<br>+         /* Create the cache */<br>+               cache = completion_cache_create(container, formatter_entry, a->word);<br>+             if (!cache) {<br>+                        ast_cli(a->fd, "Failed to create result set cache for object type %s.\n",<br>+                               formatter_entry->name);<br>+                   ao2_cleanup(container);<br>+                      return CLI_FAILURE;<br>+          }<br>+            ast_threadstorage_set_ptr(&container_cache, cache);<br>+              ao2_cleanup(container);<br>+      } else {<br>+             cache = ast_threadstorage_get_ptr(&container_cache);<br>+     }<br>+<br>+ res = cache[a->n];<br>+        if (!res) {<br>+          /* No more completions, clean up the cache */<br>+                ast_threadstorage_set_ptr(&container_cache, NULL);<br>+<br>+            /* The calling code will free our strings for us, but we still need<br>+             to clean up the array itself. */<br>+          ast_free(cache);<br>+     }<br>+<br>+ return res;<br>+}<br>+<br>+static char *sip_cli_traverse_objects_exec(<br>+     struct ast_cli_entry *e,<br>+     struct ast_cli_args *a,<br>+      struct ast_sip_cli_formatter_entry *formatter_entry,<br>+ int is_container)<br> {<br>         RAII_VAR(struct ao2_container *, container, NULL, ao2_cleanup);<br>-      RAII_VAR(struct ast_sip_cli_formatter_entry *, formatter_entry, NULL, ao2_cleanup);<br>   RAII_VAR(void *, object, NULL, ao2_cleanup);<br>- int is_container = 0;<br>-        const char *cmd1;<br>-    const char *cmd2;<br>     const char *object_id;<br>-       char formatter_type[64];<br>      const char *regex;<br> <br>         struct ast_sip_cli_context context = {<br>@@ -134,39 +189,16 @@<br>                 .recurse = 0,<br>         };<br> <br>-        if (cmd == CLI_INIT) {<br>-               return NULL;<br>- }<br>-<br>- cmd1 = e->cmda[1];<br>-        cmd2 = e->cmda[2];<br>         object_id = a->argv[3];<br> <br>-        if (!ast_ends_with(cmd2, "s")) {<br>-           ast_copy_string(formatter_type, cmd2, sizeof(formatter_type));<br>-               is_container = 0;<br>-    } else if (ast_ends_with(cmd2, "ies")) {<br>-           /* Take the plural "ies" off of the object name and re[place with "y". */<br>-                int l = strlen(cmd2);<br>-                snprintf(formatter_type, 64, "%*.*sy", l - 3, l - 3, cmd2);<br>-                is_container = 1;<br>-    } else {<br>-             /* Take the plural "s" off of the object name. */<br>-          ast_copy_string(formatter_type, cmd2, strlen(cmd2));<br>-         is_container = 1;<br>-    }<br>-<br>- if (!strcmp(cmd1, "show")) {<br>+       if (!strcmp(e->cmda[1], "show")) {<br>               context.show_details_only_level_0 = !is_container;<br>            context.recurse = 1;<br>- } else {<br>-             is_container = 1;<br>     }<br> <br>- if (cmd != CLI_GENERATE<br>-              && is_container<br>-              && a->argc >= 4<br>-                && strcmp(object_id, "like") == 0) {<br>+       if (is_container<br>+        && a->argc >= 4<br>+        && strcmp(object_id, "like") == 0) {<br>             if (ast_strlen_zero(a->argv[4])) {<br>                         return CLI_SHOWUSAGE;<br>                 }<br>@@ -175,46 +207,25 @@<br>              regex = "";<br>         }<br> <br>- if (cmd == CLI_GENERATE<br>-              && (is_container<br>-                     || a->argc > 4<br>-                 || (a->argc == 4 && ast_strlen_zero(a->word)))) {<br>-              return CLI_SUCCESS;<br>-  }<br>-<br>  context.output_buffer = ast_str_create(256);<br>  if (!context.output_buffer) {<br>                 return CLI_FAILURE;<br>   }<br> <br>- formatter_entry = ast_sip_lookup_cli_formatter(formatter_type);<br>-      if (!formatter_entry) {<br>-              ast_log(LOG_ERROR, "No formatter registered for object type %s.\n",<br>-                        formatter_type);<br>-             ast_free(context.output_buffer);<br>-             return CLI_FAILURE;<br>-  }<br>     ast_str_append(&context.output_buffer, 0, "\n");<br>        formatter_entry->print_header(NULL, &context, 0);<br>      ast_str_append(&context.output_buffer, 0,<br>                 "==========================================================================================\n\n");<br> <br>-      if (is_container || cmd == CLI_GENERATE) {<br>+   if (is_container) {<br>           container = formatter_entry->get_container(regex);<br>                 if (!container) {<br>                     ast_cli(a->fd, "No container returned for object type %s.\n",<br>-                           formatter_type);<br>+                             formatter_entry->name);<br>                    ast_free(context.output_buffer);<br>                      return CLI_FAILURE;<br>           }<br>-    }<br> <br>- if (cmd == CLI_GENERATE) {<br>-           ast_free(context.output_buffer);<br>-             return complete_show_sorcery_object(container, formatter_entry, a->word, a->n);<br>-        }<br>-<br>- if (is_container) {<br>           if (!ao2_container_count(container)) {<br>                        ast_free(context.output_buffer);<br>                      ast_cli(a->fd, "No objects found.\n\n");<br>@@ -244,6 +255,54 @@<br>   return CLI_SUCCESS;<br> }<br> <br>+char *ast_sip_cli_traverse_objects(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)<br>+{<br>+        RAII_VAR(struct ast_sip_cli_formatter_entry *, formatter_entry, NULL, ao2_cleanup);<br>+  int is_container = 0;<br>+        const char *cmd1;<br>+    const char *cmd2;<br>+    char formatter_type[64];<br>+<br>+  if (cmd == CLI_INIT) {<br>+               return NULL;<br>+ }<br>+<br>+ cmd1 = e->cmda[1];<br>+        cmd2 = e->cmda[2];<br>+<br>+     if (!ast_ends_with(cmd2, "s")) {<br>+           ast_copy_string(formatter_type, cmd2, sizeof(formatter_type));<br>+               is_container = 0;<br>+    } else if (ast_ends_with(cmd2, "ies")) {<br>+           /* Take the plural "ies" off of the object name and re[place with "y". */<br>+                int l = strlen(cmd2);<br>+                snprintf(formatter_type, 64, "%*.*sy", l - 3, l - 3, cmd2);<br>+                is_container = 1;<br>+    } else {<br>+             /* Take the plural "s" off of the object name. */<br>+          ast_copy_string(formatter_type, cmd2, strlen(cmd2));<br>+         is_container = 1;<br>+    }<br>+<br>+ if (strcmp(cmd1, "show")) {<br>+                is_container = 1;<br>+    }<br>+<br>+ formatter_entry = ast_sip_lookup_cli_formatter(formatter_type);<br>+      if (!formatter_entry) {<br>+              ast_log(LOG_ERROR, "No formatter registered for object type %s.\n",<br>+                        formatter_type);<br>+             return CLI_FAILURE;<br>+  }<br>+<br>+ switch (cmd) {<br>+       case CLI_GENERATE:<br>+           return sip_cli_traverse_objects_generate(e, a, formatter_entry, is_container);<br>+       default:<br>+             return sip_cli_traverse_objects_exec(e, a, formatter_entry, is_container);<br>+   }<br>+}<br>+<br> static int formatter_sort(const void *obj, const void *arg, int flags)<br> {<br>         const struct ast_sip_cli_formatter_entry *left_obj = obj;<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6989">change 6989</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6989"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ib6fdeb5283b7c1c66df22c0fa08e95524bf55011 </div>
<div style="display:none"> Gerrit-Change-Number: 6989 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>