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 -- [email protected] -- 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    ///

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/vim_dev/202004112032.03BKW4hd011402%40masaka.moolenaar.net.

Raspunde prin e-mail lui