To: vim_dev@googlegroups.com
Subject: Patch 8.2.4823
Fcc: outbox
From: Bram Moolenaar <Bram@moolenaar.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
------------

Patch 8.2.4823
Problem:    Concatenating more than 2 strings in a :def function is
            inefficient.
Solution:   Add a count to the CONCAT instruction. (closes #10276)
Files:      src/vim9.h, src/vim9cmds.c, src/vim9compile.c, src/vim9execute.c,
            src/vim9expr.c, src/vim9instr.c, src/proto/vim9instr.pro,
            src/testdir/test_vim9_disassemble.vim


*** ../vim-8.2.4822/src/vim9.h	2022-03-30 21:12:16.451923056 +0100
--- src/vim9.h	2022-04-25 12:30:38.844650280 +0100
***************
*** 152,158 ****
      ISN_COMPAREANY,
  
      // expression operations
!     ISN_CONCAT,
      ISN_STRINDEX,   // [expr] string index
      ISN_STRSLICE,   // [expr:expr] string slice
      ISN_LISTAPPEND, // append to a list, like add()
--- 152,158 ----
      ISN_COMPAREANY,
  
      // expression operations
!     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()
*** ../vim-8.2.4822/src/vim9cmds.c	2022-04-02 19:43:53.927491819 +0100
--- src/vim9cmds.c	2022-04-25 12:30:38.844650280 +0100
***************
*** 2125,2131 ****
  	    generate_instr_type(cctx, ISN_REDIREND, &t_string);
  
  	    if (lhs->lhs_append)
! 		generate_instr_drop(cctx, ISN_CONCAT, 1);
  
  	    if (lhs->lhs_has_index)
  	    {
--- 2125,2131 ----
  	    generate_instr_type(cctx, ISN_REDIREND, &t_string);
  
  	    if (lhs->lhs_append)
! 		generate_CONCAT(cctx, 2);
  
  	    if (lhs->lhs_has_index)
  	    {
*** ../vim-8.2.4822/src/vim9compile.c	2022-04-21 23:29:58.948561831 +0100
--- src/vim9compile.c	2022-04-25 12:37:43.708353791 +0100
***************
*** 988,1001 ****
      if (evalstr && (p = (char_u *)strstr((char *)str, "`=")) != NULL)
      {
  	char_u	*start = str;
  
  	// 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)
--- 988,999 ----
      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.
  	for (;;)
  	{
  	    if (p > start)
***************
*** 1003,1009 ****
  		// literal string before the expression
  		val = vim_strnsave(start, p - start);
  		generate_PUSHS(cctx, &val);
! 		generate_instr_drop(cctx, ISN_CONCAT, 1);
  	    }
  	    p += 2;
  
--- 1001,1007 ----
  		// literal string before the expression
  		val = vim_strnsave(start, p - start);
  		generate_PUSHS(cctx, &val);
! 		count++;
  	    }
  	    p += 2;
  
***************
*** 1011,1017 ****
  	    if (compile_expr0(&p, cctx) == FAIL)
  		return FAIL;
  	    may_generate_2STRING(-1, TRUE, cctx);
! 	    generate_instr_drop(cctx, ISN_CONCAT, 1);
  
  	    p = skipwhite(p);
  	    if (*p != '`')
--- 1009,1015 ----
  	    if (compile_expr0(&p, cctx) == FAIL)
  		return FAIL;
  	    may_generate_2STRING(-1, TRUE, cctx);
! 	    count++;
  
  	    p = skipwhite(p);
  	    if (*p != '`')
***************
*** 1029,1039 ****
  		{
  		    val = vim_strsave(start);
  		    generate_PUSHS(cctx, &val);
! 		    generate_instr_drop(cctx, ISN_CONCAT, 1);
  		}
  		break;
  	    }
  	}
      }
      else
      {
--- 1027,1040 ----
  		{
  		    val = vim_strsave(start);
  		    generate_PUSHS(cctx, &val);
! 		    count++;
  		}
  		break;
  	    }
  	}
+ 
+ 	if (count > 1)
+ 	    generate_CONCAT(cctx, count);
      }
      else
      {
***************
*** 2382,2388 ****
  
  	    if (*op == '.')
  	    {
! 		if (generate_instr_drop(cctx, ISN_CONCAT, 1) == NULL)
  		    goto theend;
  	    }
  	    else if (*op == '+')
--- 2383,2389 ----
  
  	    if (*op == '.')
  	    {
! 		if (generate_CONCAT(cctx, 2) == FAIL)
  		    goto theend;
  	    }
  	    else if (*op == '+')
*** ../vim-8.2.4822/src/vim9execute.c	2022-04-15 20:50:13.463235819 +0100
--- src/vim9execute.c	2022-04-25 12:40:20.660175789 +0100
***************
*** 120,125 ****
--- 120,167 ----
  }
  
  /*
+  * 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.
   * When "count" is -1 a NULL list is added to the stack.
***************
*** 3536,3541 ****
--- 3578,3588 ----
  		}
  		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,4362 ****
  		}
  		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:
  		{
--- 4390,4395 ----
***************
*** 6083,6089 ****
  	    case ISN_ADDBLOB: smsg("%s%4d ADDBLOB", pfx, current); break;
  
  	    // expression operations
! 	    case ISN_CONCAT: smsg("%s%4d CONCAT", pfx, current); 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;
--- 6116,6125 ----
  	    case ISN_ADDBLOB: smsg("%s%4d ADDBLOB", pfx, current); break;
  
  	    // expression operations
! 	    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;
*** ../vim-8.2.4822/src/vim9expr.c	2022-04-02 21:59:02.481697389 +0100
--- src/vim9expr.c	2022-04-25 12:30:38.848650279 +0100
***************
*** 2513,2519 ****
  		if (may_generate_2STRING(-2, FALSE, cctx) == FAIL
  			|| may_generate_2STRING(-1, FALSE, cctx) == FAIL)
  		    return FAIL;
! 		generate_instr_drop(cctx, ISN_CONCAT, 1);
  	    }
  	    else
  		generate_two_op(cctx, op);
--- 2513,2520 ----
  		if (may_generate_2STRING(-2, FALSE, cctx) == FAIL
  			|| may_generate_2STRING(-1, FALSE, cctx) == FAIL)
  		    return FAIL;
! 		if (generate_CONCAT(cctx, 2) == FAIL)
! 		    return FAIL;
  	    }
  	    else
  		generate_two_op(cctx, op);
*** ../vim-8.2.4822/src/vim9instr.c	2022-03-31 20:02:52.422045605 +0100
--- src/vim9instr.c	2022-04-25 12:30:38.848650279 +0100
***************
*** 472,477 ****
--- 472,504 ----
  }
  
  /*
+  * 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.
   */
*** ../vim-8.2.4822/src/proto/vim9instr.pro	2022-03-30 21:12:16.451923056 +0100
--- src/proto/vim9instr.pro	2022-04-25 12:30:38.844650280 +0100
***************
*** 62,67 ****
--- 62,68 ----
  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);
*** ../vim-8.2.4822/src/testdir/test_vim9_disassemble.vim	2022-04-02 21:59:02.481697389 +0100
--- src/testdir/test_vim9_disassemble.vim	2022-04-25 12:30:38.844650280 +0100
***************
*** 208,214 ****
          ' redir END\_s*' ..
          '\d LOAD $0\_s*' ..
          '\d REDIR END\_s*' ..
!         '\d CONCAT\_s*' ..
          '\d STORE $0\_s*' ..
          '\d RETURN void',
          res)
--- 208,214 ----
          ' redir END\_s*' ..
          '\d LOAD $0\_s*' ..
          '\d REDIR END\_s*' ..
!         '\d CONCAT size 2\_s*' ..
          '\d STORE $0\_s*' ..
          '\d RETURN void',
          res)
***************
*** 883,889 ****
          'local ..= arg\_s*' ..
          '\d LOADOUTER level 1 $0\_s*' ..
          '\d LOAD arg\[-1\]\_s*' ..
!         '\d CONCAT\_s*' ..
          '\d STOREOUTER level 1 $0\_s*' ..
          '\d RETURN void',
          res)
--- 883,889 ----
          'local ..= arg\_s*' ..
          '\d LOADOUTER level 1 $0\_s*' ..
          '\d LOAD arg\[-1\]\_s*' ..
!         '\d CONCAT size 2\_s*' ..
          '\d STOREOUTER level 1 $0\_s*' ..
          '\d RETURN void',
          res)
***************
*** 973,979 ****
          '6 LOAD arg\[-2]\_s*' ..
          '\d LOAD arg\[-1]\_s*' ..
          '\d 2STRING stack\[-1]\_s*' ..
!         '\d\+ CONCAT\_s*' ..
          '\d\+ RETURN',
          res)
  enddef
--- 973,979 ----
          '6 LOAD arg\[-2]\_s*' ..
          '\d LOAD arg\[-1]\_s*' ..
          '\d 2STRING stack\[-1]\_s*' ..
!         '\d\+ CONCAT size 2\_s*' ..
          '\d\+ RETURN',
          res)
  enddef
***************
*** 1245,1253 ****
          '\d PUSHS "X"\_s*' ..
          '\d LOAD arg\[-1\]\_s*' ..
          '\d 2STRING_ANY stack\[-1\]\_s*' ..
!         '\d CONCAT\_s*' ..
          '\d PUSHS "X"\_s*' ..
!         '\d CONCAT\_s*' ..
          '\d RETURN',
          instr)
  enddef
--- 1245,1253 ----
          '\d PUSHS "X"\_s*' ..
          '\d LOAD arg\[-1\]\_s*' ..
          '\d 2STRING_ANY stack\[-1\]\_s*' ..
!         '\d CONCAT size 2\_s*' ..
          '\d PUSHS "X"\_s*' ..
!         '\d CONCAT size 2\_s*' ..
          '\d RETURN',
          instr)
  enddef
***************
*** 1432,1438 ****
          '\d\+ LOAD $0\_s*' ..
          '\d\+ LOAD $2\_s*' ..
          '\d 2STRING_ANY stack\[-1\]\_s*' ..
!         '\d\+ CONCAT\_s*' ..
          '\d\+ STORE $0\_s*' ..
          'endfor\_s*' ..
          '\d\+ JUMP -> 5\_s*' ..
--- 1432,1438 ----
          '\d\+ LOAD $0\_s*' ..
          '\d\+ LOAD $2\_s*' ..
          '\d 2STRING_ANY stack\[-1\]\_s*' ..
!         '\d\+ CONCAT size 2\_s*' ..
          '\d\+ STORE $0\_s*' ..
          'endfor\_s*' ..
          '\d\+ JUMP -> 5\_s*' ..
***************
*** 2142,2148 ****
          "execute 'help ' .. tag\\_s*" ..
          '\d\+ PUSHS "help "\_s*' ..
          '\d\+ LOAD $1\_s*' ..
!         '\d\+ CONCAT\_s*' ..
          '\d\+ EXECUTE 1\_s*' ..
          '\d\+ RETURN void',
          res)
--- 2142,2148 ----
          "execute 'help ' .. tag\\_s*" ..
          '\d\+ PUSHS "help "\_s*' ..
          '\d\+ LOAD $1\_s*' ..
!         '\d\+ CONCAT size 2\_s*' ..
          '\d\+ EXECUTE 1\_s*' ..
          '\d\+ RETURN void',
          res)
*** ../vim-8.2.4822/src/version.c	2022-04-24 21:54:56.061612118 +0100
--- src/version.c	2022-04-25 12:36:00.756428126 +0100
***************
*** 748,749 ****
--- 748,751 ----
  {   /* Add new patch number below this line */
+ /**/
+     4823,
  /**/

-- 
hundred-and-one symptoms of being an internet addict:
48. You get a tatoo that says "This body best viewed with Netscape 3.1 or
    higher."

 /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net   \\\
///                                                                      \\\
\\\        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///