To: vim_dev@googlegroups.com Subject: Patch 8.1.1007 Fcc: outbox From: Bram Moolenaar 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!\") + 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 %\") + 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 %\") + 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 %\") + 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 ///