path: root/data/vim/patches/8.1.1007

To: vim_dev@googlegroups.com
Subject: Patch 8.1.1007
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.1.1007
Problem:    Using closure may consume a lot of memory.
Solution:   unreference items that are no longer needed. Add a test. (Ozaki
            Kiichi, closes #3961)
Files:	    src/testdir/Make_all.mak, src/testdir/test_memory_usage.vim,
            src/userfunc.c


*** ../vim-8.1.1006/src/testdir/Make_all.mak	2019-03-02 06:41:34.345330494 +0100
--- src/testdir/Make_all.mak	2019-03-14 13:22:28.717839506 +0100
***************
*** 63,70 ****
  # Individual tests, including the ones part of test_alot.
  # Please keep sorted up to test_alot.
  NEW_TESTS = \
- 	test_arglist \
  	test_arabic \
  	test_assert \
  	test_assign \
  	test_autochdir \
--- 63,70 ----
  # Individual tests, including the ones part of test_alot.
  # Please keep sorted up to test_alot.
  NEW_TESTS = \
  	test_arabic \
+ 	test_arglist \
  	test_assert \
  	test_assign \
  	test_autochdir \
***************
*** 108,118 ****
  	test_ex_equal \
  	test_ex_undo \
  	test_ex_z \
- 	test_exit \
  	test_exec_while_if \
  	test_execute_func \
  	test_exists \
  	test_exists_autocmd \
  	test_expand \
  	test_expand_dllpath \
  	test_expand_func \
--- 108,118 ----
  	test_ex_equal \
  	test_ex_undo \
  	test_ex_z \
  	test_exec_while_if \
  	test_execute_func \
  	test_exists \
  	test_exists_autocmd \
+ 	test_exit \
  	test_expand \
  	test_expand_dllpath \
  	test_expand_func \
***************
*** 179,184 ****
--- 179,185 ----
  	test_match \
  	test_matchadd_conceal \
  	test_matchadd_conceal_utf8 \
+ 	test_memory_usage \
  	test_menu \
  	test_messages \
  	test_mksession \
***************
*** 355,360 ****
--- 356,362 ----
  	test_maparg.res \
  	test_marks.res \
  	test_matchadd_conceal.res \
+ 	test_memory_usage.res \
  	test_mksession.res \
  	test_nested_function.res \
  	test_netbeans.res \
*** ../vim-8.1.1006/src/testdir/test_memory_usage.vim	2019-03-14 13:42:48.909493098 +0100
--- src/testdir/test_memory_usage.vim	2019-03-14 13:26:00.264275955 +0100
***************
*** 0 ****
--- 1,144 ----
+ " Tests for memory usage.
+ 
+ if !has('terminal') || has('gui_running') || $ASAN_OPTIONS !=# ''
+   " Skip tests on Travis CI ASAN build because it's difficult to estimate
+   " memory usage.
+   finish
+ endif
+ 
+ source shared.vim
+ 
+ func s:pick_nr(str) abort
+   return substitute(a:str, '[^0-9]', '', 'g') * 1
+ endfunc
+ 
+ if has('win32')
+   if !executable('wmic')
+     finish
+   endif
+   func s:memory_usage(pid) abort
+     let cmd = printf('wmic process where processid=%d get WorkingSetSize', a:pid)
+     return s:pick_nr(system(cmd)) / 1024
+   endfunc
+ elseif has('unix')
+   if !executable('ps')
+     finish
+   endif
+   func s:memory_usage(pid) abort
+     return s:pick_nr(system('ps -o rss= -p ' . a:pid))
+   endfunc
+ else
+   finish
+ endif
+ 
+ " Wait for memory usage to level off.
+ func s:monitor_memory_usage(pid) abort
+   let proc = {}
+   let proc.pid = a:pid
+   let proc.hist = []
+   let proc.min = 0
+   let proc.max = 0
+ 
+   func proc.op() abort
+     " Check the last 200ms.
+     let val = s:memory_usage(self.pid)
+     if self.min > val
+       let self.min = val
+     elseif self.max < val
+       let self.max = val
+     endif
+     call add(self.hist, val)
+     if len(self.hist) < 20
+       return 0
+     endif
+     let sample = remove(self.hist, 0)
+     return len(uniq([sample] + self.hist)) == 1
+   endfunc
+ 
+   call WaitFor({-> proc.op()}, 10000)
+   return {'last': get(proc.hist, -1), 'min': proc.min, 'max': proc.max}
+ endfunc
+ 
+ let s:term_vim = {}
+ 
+ func s:term_vim.start(...) abort
+   let self.buf = term_start([GetVimProg()] + a:000)
+   let self.job = term_getjob(self.buf)
+   call WaitFor({-> job_status(self.job) ==# 'run'})
+   let self.pid = job_info(self.job).process
+ endfunc
+ 
+ func s:term_vim.stop() abort
+   call term_sendkeys(self.buf, ":qall!\<CR>")
+   call WaitFor({-> job_status(self.job) ==# 'dead'})
+   exe self.buf . 'bwipe!'
+ endfunc
+ 
+ func s:vim_new() abort
+   return copy(s:term_vim)
+ endfunc
+ 
+ func Test_memory_func_capture_vargs()
+   " Case: if a local variable captures a:000, funccall object will be free
+   " just after it finishes.
+   let testfile = 'Xtest.vim'
+   call writefile([
+         \ 'func s:f(...)',
+         \ '  let x = a:000',
+         \ 'endfunc',
+         \ 'for _ in range(10000)',
+         \ '  call s:f(0)',
+         \ 'endfor',
+         \ ], testfile)
+ 
+   let vim = s:vim_new()
+   call vim.start('--clean', '-c', 'set noswapfile', testfile)
+   let before = s:monitor_memory_usage(vim.pid).last
+ 
+   call term_sendkeys(vim.buf, ":so %\<CR>")
+   call WaitFor({-> term_getcursor(vim.buf)[0] == 1})
+   let after = s:monitor_memory_usage(vim.pid)
+ 
+   " Estimate the limit of max usage as 2x initial usage.
+   call assert_inrange(before, 2 * before, after.max)
+   " In this case, garbase collecting is not needed.
+   call assert_equal(after.last, after.max)
+ 
+   call vim.stop()
+   call delete(testfile)
+ endfunc
+ 
+ func Test_memory_func_capture_lvars()
+   " Case: if a local variable captures l: dict, funccall object will not be
+   " free until garbage collector runs, but after that memory usage doesn't
+   " increase so much even when rerun Xtest.vim since system memory caches.
+   let testfile = 'Xtest.vim'
+   call writefile([
+         \ 'func s:f()',
+         \ '  let x = l:',
+         \ 'endfunc',
+         \ 'for _ in range(10000)',
+         \ '  call s:f()',
+         \ 'endfor',
+         \ ], testfile)
+ 
+   let vim = s:vim_new()
+   call vim.start('--clean', '-c', 'set noswapfile', testfile)
+   let before = s:monitor_memory_usage(vim.pid).last
+ 
+   call term_sendkeys(vim.buf, ":so %\<CR>")
+   call WaitFor({-> term_getcursor(vim.buf)[0] == 1})
+   let after = s:monitor_memory_usage(vim.pid)
+ 
+   " Rerun Xtest.vim.
+   for _ in range(3)
+     call term_sendkeys(vim.buf, ":so %\<CR>")
+     call WaitFor({-> term_getcursor(vim.buf)[0] == 1})
+     let last = s:monitor_memory_usage(vim.pid).last
+   endfor
+ 
+   call assert_inrange(before, after.max + (after.last - before), last)
+ 
+   call vim.stop()
+   call delete(testfile)
+ endfunc
*** ../vim-8.1.1006/src/userfunc.c	2019-02-14 13:43:33.779220100 +0100
--- src/userfunc.c	2019-03-14 13:35:20.756552196 +0100
***************
*** 39,50 ****
  /* Used by get_func_tv() */
  static garray_T funcargs = GA_EMPTY;
  
! /* pointer to funccal for currently active function */
! funccall_T *current_funccal = NULL;
  
! /* Pointer to list of previously used funccal, still around because some
!  * item in it is still being used. */
! funccall_T *previous_funccal = NULL;
  
  static char *e_funcexts = N_("E122: Function %s already exists, add ! to replace it");
  static char *e_funcdict = N_("E717: Dictionary entry already exists");
--- 39,50 ----
  /* Used by get_func_tv() */
  static garray_T funcargs = GA_EMPTY;
  
! // pointer to funccal for currently active function
! static funccall_T *current_funccal = NULL;
  
! // Pointer to list of previously used funccal, still around because some
! // item in it is still being used.
! static funccall_T *previous_funccal = NULL;
  
  static char *e_funcexts = N_("E122: Function %s already exists, add ! to replace it");
  static char *e_funcdict = N_("E717: Dictionary entry already exists");
***************
*** 586,628 ****
  }
  
  /*
!  * Free "fc" and what it contains.
   */
!    static void
! free_funccal(
!     funccall_T	*fc,
!     int		free_val)  /* a: vars were allocated */
  {
!     listitem_T	*li;
!     int		i;
  
      for (i = 0; i < fc->fc_funcs.ga_len; ++i)
      {
! 	ufunc_T	    *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i];
  
! 	/* When garbage collecting a funccall_T may be freed before the
! 	 * function that references it, clear its uf_scoped field.
! 	 * The function may have been redefined and point to another
! 	 * funccall_T, don't clear it then. */
  	if (fp != NULL && fp->uf_scoped == fc)
  	    fp->uf_scoped = NULL;
      }
      ga_clear(&fc->fc_funcs);
  
!     /* The a: variables typevals may not have been allocated, only free the
!      * allocated variables. */
!     vars_clear_ext(&fc->l_avars.dv_hashtab, free_val);
  
!     /* free all l: variables */
      vars_clear(&fc->l_vars.dv_hashtab);
  
!     /* Free the a:000 variables if they were allocated. */
!     if (free_val)
! 	for (li = fc->l_varlist.lv_first; li != NULL; li = li->li_next)
! 	    clear_tv(&li->li_tv);
  
!     func_ptr_unref(fc->func);
!     vim_free(fc);
  }
  
  /*
--- 586,636 ----
  }
  
  /*
!  * Free "fc".
   */
!     static void
! free_funccal(funccall_T *fc)
  {
!     int	i;
  
      for (i = 0; i < fc->fc_funcs.ga_len; ++i)
      {
! 	ufunc_T *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i];
  
! 	// When garbage collecting a funccall_T may be freed before the
! 	// function that references it, clear its uf_scoped field.
! 	// The function may have been redefined and point to another
! 	// funccall_T, don't clear it then.
  	if (fp != NULL && fp->uf_scoped == fc)
  	    fp->uf_scoped = NULL;
      }
      ga_clear(&fc->fc_funcs);
  
!     func_ptr_unref(fc->func);
!     vim_free(fc);
! }
  
! /*
!  * Free "fc" and what it contains.
!  * Can be called only when "fc" is kept beyond the period of it called,
!  * i.e. after cleanup_function_call(fc).
!  */
!    static void
! free_funccal_contents(funccall_T *fc)
! {
!     listitem_T	*li;
! 
!     // Free all l: variables.
      vars_clear(&fc->l_vars.dv_hashtab);
  
!     // Free all a: variables.
!     vars_clear(&fc->l_avars.dv_hashtab);
  
!     // Free the a:000 variables.
!     for (li = fc->l_varlist.lv_first; li != NULL; li = li->li_next)
! 	clear_tv(&li->li_tv);
! 
!     free_funccal(fc);
  }
  
  /*
***************
*** 632,682 ****
      static void
  cleanup_function_call(funccall_T *fc)
  {
      current_funccal = fc->caller;
  
!     /* If the a:000 list and the l: and a: dicts are not referenced and there
!      * is no closure using it, we can free the funccall_T and what's in it. */
!     if (fc->l_varlist.lv_refcount == DO_NOT_FREE_CNT
! 	    && fc->l_vars.dv_refcount == DO_NOT_FREE_CNT
! 	    && fc->l_avars.dv_refcount == DO_NOT_FREE_CNT
! 	    && fc->fc_refcount <= 0)
!     {
! 	free_funccal(fc, FALSE);
!     }
      else
      {
! 	hashitem_T	*hi;
! 	listitem_T	*li;
! 	int		todo;
! 	dictitem_T	*v;
! 	static int	made_copy = 0;
! 
! 	/* "fc" is still in use.  This can happen when returning "a:000",
! 	 * assigning "l:" to a global variable or defining a closure.
! 	 * Link "fc" in the list for garbage collection later. */
! 	fc->caller = previous_funccal;
! 	previous_funccal = fc;
  
! 	/* Make a copy of the a: variables, since we didn't do that above. */
  	todo = (int)fc->l_avars.dv_hashtab.ht_used;
  	for (hi = fc->l_avars.dv_hashtab.ht_array; todo > 0; ++hi)
  	{
  	    if (!HASHITEM_EMPTY(hi))
  	    {
  		--todo;
! 		v = HI2DI(hi);
! 		copy_tv(&v->di_tv, &v->di_tv);
  	    }
  	}
  
! 	/* Make a copy of the a:000 items, since we didn't do that above. */
  	for (li = fc->l_varlist.lv_first; li != NULL; li = li->li_next)
  	    copy_tv(&li->li_tv, &li->li_tv);
  
! 	if (++made_copy == 10000)
  	{
! 	    // We have made a lot of copies.  This can happen when
! 	    // repetitively calling a function that creates a reference to
  	    // itself somehow.  Call the garbage collector soon to avoid using
  	    // too much memory.
  	    made_copy = 0;
--- 640,714 ----
      static void
  cleanup_function_call(funccall_T *fc)
  {
+     int	may_free_fc = fc->fc_refcount <= 0;
+     int	free_fc = TRUE;
+ 
      current_funccal = fc->caller;
  
!     // Free all l: variables if not referred.
!     if (may_free_fc && fc->l_vars.dv_refcount == DO_NOT_FREE_CNT)
! 	vars_clear(&fc->l_vars.dv_hashtab);
!     else
! 	free_fc = FALSE;
! 
!     // If the a:000 list and the l: and a: dicts are not referenced and
!     // there is no closure using it, we can free the funccall_T and what's
!     // in it.
!     if (may_free_fc && fc->l_avars.dv_refcount == DO_NOT_FREE_CNT)
! 	vars_clear_ext(&fc->l_avars.dv_hashtab, FALSE);
      else
      {
! 	int	    todo;
! 	hashitem_T  *hi;
! 	dictitem_T  *di;
  
! 	free_fc = FALSE;
! 
! 	// Make a copy of the a: variables, since we didn't do that above.
  	todo = (int)fc->l_avars.dv_hashtab.ht_used;
  	for (hi = fc->l_avars.dv_hashtab.ht_array; todo > 0; ++hi)
  	{
  	    if (!HASHITEM_EMPTY(hi))
  	    {
  		--todo;
! 		di = HI2DI(hi);
! 		copy_tv(&di->di_tv, &di->di_tv);
  	    }
  	}
+     }
+ 
+     if (may_free_fc && fc->l_varlist.lv_refcount == DO_NOT_FREE_CNT)
+ 	fc->l_varlist.lv_first = NULL;
+     else
+     {
+ 	listitem_T *li;
  
! 	free_fc = FALSE;
! 
! 	// Make a copy of the a:000 items, since we didn't do that above.
  	for (li = fc->l_varlist.lv_first; li != NULL; li = li->li_next)
  	    copy_tv(&li->li_tv, &li->li_tv);
+     }
+ 
+     if (free_fc)
+ 	free_funccal(fc);
+     else
+     {
+ 	static int made_copy = 0;
  
! 	// "fc" is still in use.  This can happen when returning "a:000",
! 	// assigning "l:" to a global variable or defining a closure.
! 	// Link "fc" in the list for garbage collection later.
! 	fc->caller = previous_funccal;
! 	previous_funccal = fc;
! 
! 	if (want_garbage_collect)
! 	    // If garbage collector is ready, clear count.
! 	    made_copy = 0;
! 	else if (++made_copy >= (int)((4096 * 1024) / sizeof(*fc)))
  	{
! 	    // We have made a lot of copies, worth 4 Mbyte.  This can happen
! 	    // when repetitively calling a function that creates a reference to
  	    // itself somehow.  Call the garbage collector soon to avoid using
  	    // too much memory.
  	    made_copy = 0;
***************
*** 731,737 ****
  
      line_breakcheck();		/* check for CTRL-C hit */
  
!     fc = (funccall_T *)alloc(sizeof(funccall_T));
      if (fc == NULL)
  	return;
      fc->caller = current_funccal;
--- 763,769 ----
  
      line_breakcheck();		/* check for CTRL-C hit */
  
!     fc = (funccall_T *)alloc_clear(sizeof(funccall_T));
      if (fc == NULL)
  	return;
      fc->caller = current_funccal;
***************
*** 832,847 ****
  	{
  	    v = &fc->fixvar[fixvar_idx++].var;
  	    v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;
  	}
  	else
  	{
! 	    v = (dictitem_T *)alloc((unsigned)(sizeof(dictitem_T)
! 							     + STRLEN(name)));
  	    if (v == NULL)
  		break;
! 	    v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX | DI_FLAGS_ALLOC;
  	}
- 	STRCPY(v->di_key, name);
  
  	/* Note: the values are copied directly to avoid alloc/free.
  	 * "argvars" must have VAR_FIXED for v_lock. */
--- 864,878 ----
  	{
  	    v = &fc->fixvar[fixvar_idx++].var;
  	    v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX;
+ 	    STRCPY(v->di_key, name);
  	}
  	else
  	{
! 	    v = dictitem_alloc(name);
  	    if (v == NULL)
  		break;
! 	    v->di_flags |= DI_FLAGS_RO | DI_FLAGS_FIX;
  	}
  
  	/* Note: the values are copied directly to avoid alloc/free.
  	 * "argvars" must have VAR_FIXED for v_lock. */
***************
*** 860,868 ****
  
  	if (ai >= 0 && ai < MAX_FUNC_ARGS)
  	{
! 	    list_append(&fc->l_varlist, &fc->l_listitems[ai]);
! 	    fc->l_listitems[ai].li_tv = argvars[i];
! 	    fc->l_listitems[ai].li_tv.v_lock = VAR_FIXED;
  	}
      }
  
--- 891,901 ----
  
  	if (ai >= 0 && ai < MAX_FUNC_ARGS)
  	{
! 	    listitem_T *li = &fc->l_listitems[ai];
! 
! 	    li->li_tv = argvars[i];
! 	    li->li_tv.v_lock = VAR_FIXED;
! 	    list_append(&fc->l_varlist, li);
  	}
      }
  
***************
*** 1088,1094 ****
  	    if (fc == *pfc)
  	    {
  		*pfc = fc->caller;
! 		free_funccal(fc, TRUE);
  		return;
  	    }
  	}
--- 1121,1127 ----
  	    if (fc == *pfc)
  	    {
  		*pfc = fc->caller;
! 		free_funccal_contents(fc);
  		return;
  	    }
  	}
***************
*** 3646,3652 ****
  	{
  	    fc = *pfc;
  	    *pfc = fc->caller;
! 	    free_funccal(fc, TRUE);
  	    did_free = TRUE;
  	    did_free_funccal = TRUE;
  	}
--- 3679,3685 ----
  	{
  	    fc = *pfc;
  	    *pfc = fc->caller;
! 	    free_funccal_contents(fc);
  	    did_free = TRUE;
  	    did_free_funccal = TRUE;
  	}
*** ../vim-8.1.1006/src/version.c	2019-03-13 06:49:20.492351919 +0100
--- src/version.c	2019-03-14 13:27:08.671770246 +0100
***************
*** 781,782 ****
--- 781,784 ----
  {   /* Add new patch number below this line */
+ /**/
+     1007,
  /**/

-- 
"I don’t know how to make a screenshot" - Richard Stallman, July 2002
(when asked to send a screenshot of his desktop for unix.se)

 /// 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    ///