From 372bcceeee8012ef3fb2f3dbc8132c3a33cb84fc Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 25 Apr 2022 12:43:20 +0100 Subject: [PATCH] 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) --- src/proto/vim9instr.pro | 1 + src/testdir/test_vim9_disassemble.vim | 14 +++--- src/version.c | 2 + src/vim9.h | 2 +- src/vim9cmds.c | 2 +- src/vim9compile.c | 15 +++--- src/vim9execute.c | 66 +++++++++++++++++++++------ src/vim9expr.c | 3 +- src/vim9instr.c | 27 +++++++++++ 9 files changed, 100 insertions(+), 32 deletions(-) diff --git a/src/proto/vim9instr.pro b/src/proto/vim9instr.pro index 535673c0f..76c3f882a 100644 --- 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 f040ba6fd..1b2bd03ed 100644 --- 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 612d9e060..b060d5eac 100644 --- 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 345060af9..d759be209 100644 --- 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 a853d9b0b..ad32c32ff 100644 --- 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 88a4d57d7..c1d5fb95b 100644 --- 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 `=` 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 afa0dcf19..bf55d18cd 100644 --- 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 69f670b24..efa2ef597 100644 --- 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 c89f5274b..0cc1c7189 100644 --- 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. -- 2.40.0