Patch 8.2.0546

[Bram Moolenaar]

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 -- Br...@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 ///