patch 8.2.4823: concat more than 2 strings in :def function is inefficient

Problem:    Concatenating more than 2 strings in a :def function is
            inefficient.
Solution:   Add a count to the CONCAT instruction. (closes #10276)

diff --git a/src/proto/vim9instr.pro b/src/proto/vim9instr.pro
index 535673c0fd79f6eb7c7d7db6483adb6cb216f0ae..76c3f882a5e532f54435174f3c430e9b73354290 100644 (file)
--- a/src/proto/vim9instr.pro
+++ b/src/proto/vim9instr.pro
@@ -62,6 +62,7 @@ int generate_LEGACY_EVAL(cctx_T *cctx, char_u *line);
 int generate_EXECCONCAT(cctx_T *cctx, int count);
 int generate_RANGE(cctx_T *cctx, char_u *range);
 int generate_UNPACK(cctx_T *cctx, int var_count, int semicolon);
+int generate_CONCAT(cctx_T *cctx, int count);
 int generate_cmdmods(cctx_T *cctx, cmdmod_T *cmod);
 int generate_undo_cmdmods(cctx_T *cctx);
 int generate_store_var(cctx_T *cctx, assign_dest_T dest, int opt_flags, int vimvaridx, int scriptvar_idx, int scriptvar_sid, type_T *type, char_u *name);

diff --git a/src/testdir/test_vim9_disassemble.vim b/src/testdir/test_vim9_disassemble.vim
index f040ba6fd09c39559d1e249e43f8b4c44bbc81af..1b2bd03ed764886e12726b8aa55e890cd7c83a70 100644 (file)
--- a/src/testdir/test_vim9_disassemble.vim
+++ b/src/testdir/test_vim9_disassemble.vim
@@ -208,7 +208,7 @@ def Test_disassemble_redir_var()
         ' redir END\_s*' ..
         '\d LOAD $0\_s*' ..
         '\d REDIR END\_s*' ..
-        '\d CONCAT\_s*' ..
+        '\d CONCAT size 2\_s*' ..
         '\d STORE $0\_s*' ..
         '\d RETURN void',
         res)
@@ -883,7 +883,7 @@ def Test_disassemble_closure()
         'local ..= arg\_s*' ..
         '\d LOADOUTER level 1 $0\_s*' ..
         '\d LOAD arg\[-1\]\_s*' ..
-        '\d CONCAT\_s*' ..
+        '\d CONCAT size 2\_s*' ..
         '\d STOREOUTER level 1 $0\_s*' ..
         '\d RETURN void',
         res)
@@ -973,7 +973,7 @@ def Test_disassemble_call_default()
         '6 LOAD arg\[-2]\_s*' ..
         '\d LOAD arg\[-1]\_s*' ..
         '\d 2STRING stack\[-1]\_s*' ..
-        '\d\+ CONCAT\_s*' ..
+        '\d\+ CONCAT size 2\_s*' ..
         '\d\+ RETURN',
         res)
 enddef
@@ -1245,9 +1245,9 @@ def Test_disassemble_lambda()
         '\d PUSHS "X"\_s*' ..
         '\d LOAD arg\[-1\]\_s*' ..
         '\d 2STRING_ANY stack\[-1\]\_s*' ..
-        '\d CONCAT\_s*' ..
+        '\d CONCAT size 2\_s*' ..
         '\d PUSHS "X"\_s*' ..
-        '\d CONCAT\_s*' ..
+        '\d CONCAT size 2\_s*' ..
         '\d RETURN',
         instr)
 enddef
@@ -1432,7 +1432,7 @@ def Test_disassemble_for_loop_eval()
         '\d\+ LOAD $0\_s*' ..
         '\d\+ LOAD $2\_s*' ..
         '\d 2STRING_ANY stack\[-1\]\_s*' ..
-        '\d\+ CONCAT\_s*' ..
+        '\d\+ CONCAT size 2\_s*' ..
         '\d\+ STORE $0\_s*' ..
         'endfor\_s*' ..
         '\d\+ JUMP -> 5\_s*' ..
@@ -2142,7 +2142,7 @@ def Test_disassemble_execute()
         "execute 'help ' .. tag\\_s*" ..
         '\d\+ PUSHS "help "\_s*' ..
         '\d\+ LOAD $1\_s*' ..
-        '\d\+ CONCAT\_s*' ..
+        '\d\+ CONCAT size 2\_s*' ..
         '\d\+ EXECUTE 1\_s*' ..
         '\d\+ RETURN void',
         res)

diff --git a/src/version.c b/src/version.c
index 612d9e06006a9cbf64e23a070153a647bd7894a4..b060d5eac07d2df79a16c7515e7f40577183bd93 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -746,6 +746,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    4823,
 /**/
     4822,
 /**/

diff --git a/src/vim9.h b/src/vim9.h
index 345060af9a3c9aeec260b6c1b367f708df325fe4..d759be2090593be2155df4e488cf4a2332e745f3 100644 (file)
--- a/src/vim9.h
+++ b/src/vim9.h
@@ -152,7 +152,7 @@ typedef enum {
     ISN_COMPAREANY,
 
     // expression operations
-    ISN_CONCAT,
+    ISN_CONCAT,     // concatenate isn_arg.number strings
     ISN_STRINDEX,   // [expr] string index
     ISN_STRSLICE,   // [expr:expr] string slice
     ISN_LISTAPPEND, // append to a list, like add()

diff --git a/src/vim9cmds.c b/src/vim9cmds.c
index a853d9b0be33dd0d55a9c08e15a6671392bf3965..ad32c32ff7cb25d30f339ad5dac830a025161c47 100644 (file)
--- a/src/vim9cmds.c
+++ b/src/vim9cmds.c
@@ -2125,7 +2125,7 @@ compile_redir(char_u *line, exarg_T *eap, cctx_T *cctx)
            generate_instr_type(cctx, ISN_REDIREND, &t_string);
 
            if (lhs->lhs_append)
-               generate_instr_drop(cctx, ISN_CONCAT, 1);
+               generate_CONCAT(cctx, 2);
 
            if (lhs->lhs_has_index)
            {

diff --git a/src/vim9compile.c b/src/vim9compile.c
index 88a4d57d7ca4ad4e13209310541d0e82a961350a..c1d5fb95bf425e1657a1cd5dfe39bda563727500 100644 (file)
--- a/src/vim9compile.c
+++ b/src/vim9compile.c
@@ -988,14 +988,12 @@ compile_heredoc_string(char_u *str, int evalstr, cctx_T *cctx)
     if (evalstr && (p = (char_u *)strstr((char *)str, "`=")) != NULL)
     {
        char_u  *start = str;
+       int     count = 0;
 
        // Need to evaluate expressions of the form `=<expr>` in the string.
        // Split the string into literal strings and Vim expressions and
        // generate instructions to concatenate the literal strings and the
        // result of evaluating the Vim expressions.
-       val = vim_strsave((char_u *)"");
-       generate_PUSHS(cctx, &val);
-
        for (;;)
        {
            if (p > start)
@@ -1003,7 +1001,7 @@ compile_heredoc_string(char_u *str, int evalstr, cctx_T *cctx)
                // literal string before the expression
                val = vim_strnsave(start, p - start);
                generate_PUSHS(cctx, &val);
-               generate_instr_drop(cctx, ISN_CONCAT, 1);
+               count++;
            }
            p += 2;
 
@@ -1011,7 +1009,7 @@ compile_heredoc_string(char_u *str, int evalstr, cctx_T *cctx)
            if (compile_expr0(&p, cctx) == FAIL)
                return FAIL;
            may_generate_2STRING(-1, TRUE, cctx);
-           generate_instr_drop(cctx, ISN_CONCAT, 1);
+           count++;
 
            p = skipwhite(p);
            if (*p != '`')
@@ -1029,11 +1027,14 @@ compile_heredoc_string(char_u *str, int evalstr, cctx_T *cctx)
                {
                    val = vim_strsave(start);
                    generate_PUSHS(cctx, &val);
-                   generate_instr_drop(cctx, ISN_CONCAT, 1);
+                   count++;
                }
                break;
            }
        }
+
+       if (count > 1)
+           generate_CONCAT(cctx, count);
     }
     else
     {
@@ -2382,7 +2383,7 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
 
            if (*op == '.')
            {
-               if (generate_instr_drop(cctx, ISN_CONCAT, 1) == NULL)
+               if (generate_CONCAT(cctx, 2) == FAIL)
                    goto theend;
            }
            else if (*op == '+')

diff --git a/src/vim9execute.c b/src/vim9execute.c
index afa0dcf198263f7090a1e28630e4d22f9c4dd7aa..bf55d18cd97aeaf8cf2a0952bbdba77ed299f6bb 100644 (file)
--- a/src/vim9execute.c
+++ b/src/vim9execute.c
@@ -119,6 +119,48 @@ ufunc_argcount(ufunc_T *ufunc)
     return ufunc->uf_args.ga_len + (ufunc->uf_va_name != NULL ? 1 : 0);
 }
 
+/*
+ * Create a new string from "count" items at the bottom of the stack.
+ * A trailing NUL is appended.
+ * When "count" is zero an empty string is added to the stack.
+ */
+    static int
+exe_concat(int count, ectx_T *ectx)
+{
+    int                idx;
+    int                len = 0;
+    typval_T   *tv;
+    garray_T   ga;
+
+    ga_init2(&ga, sizeof(char), 1);
+    // Preallocate enough space for the whole string to avoid having to grow
+    // and copy.
+    for (idx = 0; idx < count; ++idx)
+    {
+       tv = STACK_TV_BOT(idx - count);
+       if (tv->vval.v_string != NULL)
+           len += (int)STRLEN(tv->vval.v_string);
+    }
+
+    if (ga_grow(&ga, len + 1) == FAIL)
+       return FAIL;
+
+    for (idx = 0; idx < count; ++idx)
+    {
+       tv = STACK_TV_BOT(idx - count);
+       ga_concat(&ga, tv->vval.v_string);
+       clear_tv(tv);
+    }
+
+    // add a terminating NUL
+    ga_append(&ga, NUL);
+
+    ectx->ec_stack.ga_len -= count - 1;
+    STACK_TV_BOT(-1)->vval.v_string = ga.ga_data;
+
+    return OK;
+}
+
 /*
  * Create a new list from "count" items at the bottom of the stack.
  * When "count" is zero an empty list is added to the stack.
@@ -3536,6 +3578,11 @@ exec_instructions(ectx_T *ectx)
                }
                break;
 
+           case ISN_CONCAT:
+               if (exe_concat(iptr->isn_arg.number, ectx) == FAIL)
+                   goto theend;
+               break;
+
            // create a partial with NULL value
            case ISN_NEWPARTIAL:
                if (GA_GROW_FAILS(&ectx->ec_stack, 1))
@@ -4343,20 +4390,6 @@ exec_instructions(ectx_T *ectx)
                }
                break;
 
-           case ISN_CONCAT:
-               {
-                   char_u *str1 = STACK_TV_BOT(-2)->vval.v_string;
-                   char_u *str2 = STACK_TV_BOT(-1)->vval.v_string;
-                   char_u *res;
-
-                   res = concat_str(str1, str2);
-                   clear_tv(STACK_TV_BOT(-2));
-                   clear_tv(STACK_TV_BOT(-1));
-                   --ectx->ec_stack.ga_len;
-                   STACK_TV_BOT(-1)->vval.v_string = res;
-               }
-               break;
-
            case ISN_STRINDEX:
            case ISN_STRSLICE:
                {
@@ -6083,7 +6116,10 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
            case ISN_ADDBLOB: smsg("%s%4d ADDBLOB", pfx, current); break;
 
            // expression operations
-           case ISN_CONCAT: smsg("%s%4d CONCAT", pfx, current); break;
+           case ISN_CONCAT:
+               smsg("%s%4d CONCAT size %lld", pfx, current,
+                                           (varnumber_T)(iptr->isn_arg.number));
+               break;
            case ISN_STRINDEX: smsg("%s%4d STRINDEX", pfx, current); break;
            case ISN_STRSLICE: smsg("%s%4d STRSLICE", pfx, current); break;
            case ISN_BLOBINDEX: smsg("%s%4d BLOBINDEX", pfx, current); break;

diff --git a/src/vim9expr.c b/src/vim9expr.c
index 69f670b24f3af55445707b91d749969688423eb7..efa2ef59781798106b899069f7e4c39a743a1789 100644 (file)
--- a/src/vim9expr.c
+++ b/src/vim9expr.c
@@ -2513,7 +2513,8 @@ compile_expr5(char_u **arg, cctx_T *cctx, ppconst_T *ppconst)
                if (may_generate_2STRING(-2, FALSE, cctx) == FAIL
                        || may_generate_2STRING(-1, FALSE, cctx) == FAIL)
                    return FAIL;
-               generate_instr_drop(cctx, ISN_CONCAT, 1);
+               if (generate_CONCAT(cctx, 2) == FAIL)
+                   return FAIL;
            }
            else
                generate_two_op(cctx, op);

diff --git a/src/vim9instr.c b/src/vim9instr.c
index c89f5274b44d84c6cf229a8fc4fe8268cc6ada31..0cc1c718954715fb47c8323230997c84e56ece01 100644 (file)
--- a/src/vim9instr.c
+++ b/src/vim9instr.c
@@ -471,6 +471,33 @@ generate_COMPARE(cctx_T *cctx, exprtype_T exprtype, int ic)
     return OK;
 }
 
+/*
+ * Generate an ISN_CONCAT instruction.
+ * "count" is the number of stack elements to join together and it must be
+ * greater or equal to one.
+ * The caller ensures all the "count" elements on the stack have the right type.
+ */
+    int
+generate_CONCAT(cctx_T *cctx, int count)
+{
+    isn_T      *isn;
+    garray_T   *stack = &cctx->ctx_type_stack;
+
+    RETURN_OK_IF_SKIP(cctx);
+
+    if (count < 1)
+       return FAIL;
+
+    if ((isn = generate_instr(cctx, ISN_CONCAT)) == NULL)
+       return FAIL;
+    isn->isn_arg.number = count;
+
+    // drop the argument types
+    stack->ga_len -= count - 1;
+
+    return OK;
+}
+
 /*
  * Generate an ISN_2BOOL instruction.
  * "offset" is the offset in the type stack.