To: vim_dev@googlegroups.com Subject: Patch 8.2.0546 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.0546 Problem: Vim9: varargs implementation is inefficient. Solution: Create list without moving the arguments. Files: src/vim9compile.c, src/vim9execute.c *** ../vim-8.2.0545/src/vim9compile.c 2020-04-11 20:50:25.376120463 +0200 --- src/vim9compile.c 2020-04-11 21:54:16.795597595 +0200 *************** *** 5584,5598 **** ufunc->uf_def_arg_idx[count] = instr->ga_len; } - // If varargs is use, push a list. Empty if no more arguments. - if (ufunc->uf_va_name != NULL) - { - if (generate_NEWLIST(&cctx, 0) == FAIL - || generate_STORE(&cctx, ISN_STORE, - -STACK_FRAME_SIZE - 1, NULL) == FAIL) - goto erret; - } - /* * Loop over all the lines of the function and generate instructions. */ --- 5584,5589 ---- *** ../vim-8.2.0545/src/vim9execute.c 2020-04-11 20:50:25.376120463 +0200 --- src/vim9execute.c 2020-04-11 22:19:59.259991259 +0200 *************** *** 108,113 **** --- 108,142 ---- } /* + * 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. + */ + static int + exe_newlist(int count, ectx_T *ectx) + { + list_T *list = list_alloc_with_items(count); + int idx; + typval_T *tv; + + if (list == NULL) + return FAIL; + for (idx = 0; idx < count; ++idx) + list_set_item(list, idx, STACK_TV_BOT(idx - count)); + + if (count > 0) + ectx->ec_stack.ga_len -= count - 1; + else if (ga_grow(&ectx->ec_stack, 1) == FAIL) + return FAIL; + else + ++ectx->ec_stack.ga_len; + tv = STACK_TV_BOT(-1); + tv->v_type = VAR_LIST; + tv->vval.v_list = list; + ++list->lv_refcount; + return OK; + } + + /* * Call compiled function "cdf_idx" from compiled code. * * Stack has: *************** *** 137,182 **** if (ufunc->uf_va_name != NULL) { ! int iidx; ! isn_T *iptr; ! ! // Need to make a list out of the vararg arguments. There is a ! // ISN_NEWLIST instruction at the start of the function body, we need ! // to move the arguments below the stack frame and pass the count. // Stack at time of call with 2 varargs: // normal_arg // optional_arg // vararg_1 // vararg_2 ! // When starting execution: // normal_arg ! // optional_arg ! // space list of varargs ! // STACK_FRAME ! // [local variables] ! // vararg_1 ! // vararg_2 ! // TODO: This doesn't work if the same function is used for a default ! // argument value. Forbid that somehow? vararg_count = argcount - ufunc->uf_args.ga_len; if (vararg_count < 0) vararg_count = 0; else argcount -= vararg_count; ! if (ufunc->uf_def_arg_idx == NULL) ! iidx = 0; ! else ! iidx = ufunc->uf_def_arg_idx[ufunc->uf_def_args.ga_len]; ! iptr = &dfunc->df_instr[iidx]; ! if (iptr->isn_type != ISN_NEWLIST) ! { ! iemsg("Not a ISN_NEWLIST instruction"); return FAIL; ! } ! iptr->isn_arg.number = vararg_count; } ! arg_to_add = ufunc_argcount(ufunc) - argcount; if (arg_to_add < 0) { iemsg("Argument count wrong?"); --- 166,199 ---- if (ufunc->uf_va_name != NULL) { ! // Need to make a list out of the vararg arguments. // Stack at time of call with 2 varargs: // normal_arg // optional_arg // vararg_1 // vararg_2 ! // After creating the list: ! // normal_arg ! // optional_arg ! // vararg-list ! // With missing optional arguments we get: ! // normal_arg ! // After creating the list // normal_arg ! // (space for optional_arg) ! // vararg-list vararg_count = argcount - ufunc->uf_args.ga_len; if (vararg_count < 0) vararg_count = 0; else argcount -= vararg_count; ! if (exe_newlist(vararg_count, ectx) == FAIL) return FAIL; ! ! vararg_count = 1; } ! arg_to_add = ufunc->uf_args.ga_len - argcount; if (arg_to_add < 0) { iemsg("Argument count wrong?"); *************** *** 185,205 **** if (ga_grow(&ectx->ec_stack, arg_to_add + 3 + dfunc->df_varcount) == FAIL) return FAIL; ! if (vararg_count > 0) ! { ! int stack_added = arg_to_add + STACK_FRAME_SIZE + dfunc->df_varcount; ! ! // Move the varargs to below the stack frame. ! // TODO: use mch_memmove() ! for (idx = 1; idx <= vararg_count; ++idx) ! *STACK_TV_BOT(stack_added - idx) = *STACK_TV_BOT(-idx); ! ectx->ec_stack.ga_len -= vararg_count; ! } // Reserve space for omitted optional arguments, filled in soon. - // Also room for a list of varargs, if there is one. for (idx = 0; idx < arg_to_add; ++idx) ! STACK_TV_BOT(idx)->v_type = VAR_UNKNOWN; ectx->ec_stack.ga_len += arg_to_add; // Store current execution state in stack frame for ISN_RETURN. --- 202,214 ---- if (ga_grow(&ectx->ec_stack, arg_to_add + 3 + dfunc->df_varcount) == FAIL) return FAIL; ! // Move the vararg-list to below the missing optional arguments. ! if (vararg_count > 0 && arg_to_add > 0) ! *STACK_TV_BOT(arg_to_add - 1) = *STACK_TV_BOT(-1); // Reserve space for omitted optional arguments, filled in soon. for (idx = 0; idx < arg_to_add; ++idx) ! STACK_TV_BOT(idx - vararg_count)->v_type = VAR_UNKNOWN; ectx->ec_stack.ga_len += arg_to_add; // Store current execution state in stack frame for ISN_RETURN. *************** *** 213,220 **** // Initialize local variables for (idx = 0; idx < dfunc->df_varcount; ++idx) STACK_TV_BOT(STACK_FRAME_SIZE + idx)->v_type = VAR_UNKNOWN; ! ectx->ec_stack.ga_len += STACK_FRAME_SIZE + dfunc->df_varcount ! + vararg_count; // Set execution state to the start of the called function. ectx->ec_dfunc_idx = cdf_idx; --- 222,228 ---- // Initialize local variables for (idx = 0; idx < dfunc->df_varcount; ++idx) STACK_TV_BOT(STACK_FRAME_SIZE + idx)->v_type = VAR_UNKNOWN; ! ectx->ec_stack.ga_len += STACK_FRAME_SIZE + dfunc->df_varcount; // Set execution state to the start of the called function. ectx->ec_dfunc_idx = cdf_idx; *************** *** 979,1004 **** // create a list from items on the stack; uses a single allocation // for the list header and the items case ISN_NEWLIST: ! { ! int count = iptr->isn_arg.number; ! list_T *list = list_alloc_with_items(count); ! ! if (list == NULL) ! goto failed; ! for (idx = 0; idx < count; ++idx) ! list_set_item(list, idx, STACK_TV_BOT(idx - count)); ! ! if (count > 0) ! ectx.ec_stack.ga_len -= count - 1; ! else if (ga_grow(&ectx.ec_stack, 1) == FAIL) ! goto failed; ! else ! ++ectx.ec_stack.ga_len; ! tv = STACK_TV_BOT(-1); ! tv->v_type = VAR_LIST; ! tv->vval.v_list = list; ! ++list->lv_refcount; ! } break; // create a dict from items on the stack --- 987,994 ---- // create a list from items on the stack; uses a single allocation // for the list header and the items case ISN_NEWLIST: ! if (exe_newlist(iptr->isn_arg.number, &ectx) == FAIL) ! goto failed; break; // create a dict from items on the stack *** ../vim-8.2.0545/src/version.c 2020-04-11 21:42:45.052626550 +0200 --- src/version.c 2020-04-11 21:54:59.887520511 +0200 *************** *** 740,741 **** --- 740,743 ---- { /* Add new patch number below this line */ + /**/ + 546, /**/ -- A disclaimer for the disclaimer: "and before I get a huge amount of complaints , I have no control over the disclaimer at the end of this mail :-)" (Timothy Aldrich) /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///