diff options
| -rw-r--r-- | src/nvim/eval.c | 3 | ||||
| -rw-r--r-- | src/nvim/eval/typval_encode.c.h | 2 | ||||
| -rw-r--r-- | src/nvim/eval/userfunc.c | 109 | ||||
| -rw-r--r-- | test/functional/legacy/memory_usage_spec.lua | 151 |
4 files changed, 223 insertions, 42 deletions
diff --git a/src/nvim/eval.c b/src/nvim/eval.c index ac4713b983..f9e24e094d 100644 --- a/src/nvim/eval.c +++ b/src/nvim/eval.c @@ -442,7 +442,8 @@ void eval_clear(void) // unreferenced lists and dicts (void)garbage_collect(false); - // functions + // functions need to be freed before gargabe collecting, otherwise local + // variables might be freed twice. free_all_functions(); } diff --git a/src/nvim/eval/typval_encode.c.h b/src/nvim/eval/typval_encode.c.h index af21a6fbe3..94986a64b5 100644 --- a/src/nvim/eval/typval_encode.c.h +++ b/src/nvim/eval/typval_encode.c.h @@ -607,7 +607,7 @@ _convert_one_value_regular_dict: {} kMPConvDict); TYPVAL_ENCODE_CONV_DICT_START(tv, tv->vval.v_dict, tv->vval.v_dict->dv_hashtab.ht_used); - assert(saved_copyID != copyID && saved_copyID != copyID - 1); + assert(saved_copyID != copyID); _mp_push(*mpstack, ((MPConvStackVal) { .tv = tv, .type = kMPConvDict, diff --git a/src/nvim/eval/userfunc.c b/src/nvim/eval/userfunc.c index 9e123e505f..de4ca86680 100644 --- a/src/nvim/eval/userfunc.c +++ b/src/nvim/eval/userfunc.c @@ -43,11 +43,11 @@ hashtab_T func_hashtab; static garray_T funcargs = GA_EMPTY_INIT_VALUE; // pointer to funccal for currently active function -funccall_T *current_funccal = NULL; +static 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 funccall_T *previous_funccal = NULL; static char *e_funcexts = N_( "E122: Function %s already exists, add ! to replace it"); @@ -541,14 +541,8 @@ static void add_nr_var(dict_T *dp, dictitem_T *v, char *name, varnumber_T nr) v->di_tv.vval.v_number = nr; } -/* - * Free "fc" and what it contains. - */ -static void -free_funccal( - funccall_T *fc, - int free_val // a: vars were allocated -) +// Free "fc" +static void free_funccal(funccall_T *fc) { for (int i = 0; i < fc->fc_funcs.ga_len; i++) { ufunc_T *fp = ((ufunc_T **)(fc->fc_funcs.ga_data))[i]; @@ -563,56 +557,89 @@ free_funccal( } 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); + func_ptr_unref(fc->func); + xfree(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) +{ // Free all l: variables. vars_clear(&fc->l_vars.dv_hashtab); - // Free the a:000 variables if they were allocated. - if (free_val) { - TV_LIST_ITER(&fc->l_varlist, li, { - tv_clear(TV_LIST_ITEM_TV(li)); - }); - } + // Free all a: variables. + vars_clear(&fc->l_avars.dv_hashtab); - func_ptr_unref(fc->func); - xfree(fc); + // Free the a:000 variables. + TV_LIST_ITER(&fc->l_varlist, li, { + tv_clear(TV_LIST_ITEM_TV(li)); + }); + + free_funccal(fc); } /// Handle the last part of returning from a function: free the local hashtable. /// Unless it is still in use by a closure. static void cleanup_function_call(funccall_T *fc) { + bool may_free_fc = fc->fc_refcount <= 0; + bool free_fc = true; + 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_referenced(fc)) { - free_funccal(fc, false); + // 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 { - static int made_copy = 0; + free_fc = false; + } - // "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 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 { + free_fc = false; // Make a copy of the a: variables, since we didn't do that above. TV_DICT_ITER(&fc->l_avars, di, { tv_copy(&di->di_tv, &di->di_tv); }); + } + + if (may_free_fc && fc->l_varlist.lv_refcount // NOLINT(runtime/deprecated) + == DO_NOT_FREE_CNT) { + fc->l_varlist.lv_first = NULL; // NOLINT(runtime/deprecated) + + } else { + free_fc = false; // Make a copy of the a:000 items, since we didn't do that above. TV_LIST_ITER(&fc->l_varlist, li, { tv_copy(TV_LIST_ITEM_TV(li), TV_LIST_ITEM_TV(li)); }); + } - if (++made_copy == 10000) { - // We have made a lot of copies. This can happen when - // repetitively calling a function that creates a reference to + 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; @@ -639,7 +666,7 @@ static void funccal_unref(funccall_T *fc, ufunc_T *fp, bool force) for (pfc = &previous_funccal; *pfc != NULL; pfc = &(*pfc)->caller) { if (fc == *pfc) { *pfc = fc->caller; - free_funccal(fc, true); + free_funccal_contents(fc); return; } } @@ -766,7 +793,7 @@ void call_user_func(ufunc_T *fp, int argcount, typval_T *argvars, // check for CTRL-C hit line_breakcheck(); // prepare the funccall_T structure - fc = xmalloc(sizeof(funccall_T)); + fc = xcalloc(1, sizeof(funccall_T)); fc->caller = current_funccal; current_funccal = fc; fc->func = fp; @@ -881,9 +908,11 @@ void call_user_func(ufunc_T *fp, int argcount, typval_T *argvars, } if (ai >= 0 && ai < MAX_FUNC_ARGS) { - tv_list_append(&fc->l_varlist, &fc->l_listitems[ai]); - *TV_LIST_ITEM_TV(&fc->l_listitems[ai]) = argvars[i]; - TV_LIST_ITEM_TV(&fc->l_listitems[ai])->v_lock = VAR_FIXED; + listitem_T *li = &fc->l_listitems[ai]; + + *TV_LIST_ITEM_TV(li) = argvars[i]; + TV_LIST_ITEM_TV(li)->v_lock = VAR_FIXED; + tv_list_append(&fc->l_varlist, li); } } @@ -3134,7 +3163,7 @@ bool free_unref_funccal(int copyID, int testing) if (can_free_funccal(*pfc, copyID)) { funccall_T *fc = *pfc; *pfc = fc->caller; - free_funccal(fc, true); + free_funccal_contents(fc); did_free = true; did_free_funccal = true; } else { diff --git a/test/functional/legacy/memory_usage_spec.lua b/test/functional/legacy/memory_usage_spec.lua new file mode 100644 index 0000000000..14a7f2dbdd --- /dev/null +++ b/test/functional/legacy/memory_usage_spec.lua @@ -0,0 +1,151 @@ +local helpers = require('test.functional.helpers')(after_each) +local clear = helpers.clear +local eval = helpers.eval +local eq = helpers.eq +local feed_command = helpers.feed_command +local iswin = helpers.iswin +local retry = helpers.retry +local ok = helpers.ok +local source = helpers.source + +local monitor_memory_usage = { + memory_usage = function(self) + local handle + if iswin() then + handle = io.popen('wmic process where processid=' ..self.pid..' get WorkingSetSize') + else + handle = io.popen('ps -o rss= -p '..self.pid) + end + return tonumber(handle:read('*a'):match('%d+')) + end, + op = function(self) + retry(nil, 10000, function() + local val = self.memory_usage(self) + if self.min > val then + self.min = val + elseif self.max < val then + self.max = val + end + table.insert(self.hist, val) + ok(#self.hist > 20) + local result = {} + for key,value in ipairs(self.hist) do + if value ~= self.hist[key + 1] then + table.insert(result, value) + end + end + table.remove(self.hist, 1) + self.last = self.hist[#self.hist] + eq(#result, 1) + end) + end, + dump = function(self) + return 'max: '..self.max ..', last: '..self.last + end, + monitor_memory_usage = function(self, pid) + local obj = { + pid = pid, + min = 0, + max = 0, + last = 0, + hist = {}, + } + setmetatable(obj, { __index = self }) + obj:op() + return obj + end +} +setmetatable(monitor_memory_usage, +{__call = function(self, pid) + return monitor_memory_usage.monitor_memory_usage(self, pid) +end}) + +describe('memory usage', function() + local function check_result(tbl, status, result) + if not status then + print('') + for key, val in pairs(tbl) do + print(key, val:dump()) + end + error(result) + end + end + + local function isasan() + local version = eval('execute("version")') + return version:match('-fsanitize=[a-z,]*address') + end + + before_each(clear) + + --[[ + Case: if a local variable captures a:000, funccall object will be free + just after it finishes. + ]]-- + it('function capture vargs', function() + if isasan() then + pending('ASAN build is difficult to estimate memory usage') + end + if iswin() and eval("executable('wmic')") == 0 then + pending('missing "wmic" command') + elseif eval("executable('ps')") == 0 then + pending('missing "ps" command') + end + + local pid = eval('getpid()') + local before = monitor_memory_usage(pid) + source([[ + func s:f(...) + let x = a:000 + endfunc + for _ in range(10000) + call s:f(0) + endfor + ]]) + local after = monitor_memory_usage(pid) + -- Estimate the limit of max usage as 2x initial usage. + check_result({before=before, after=after}, pcall(ok, before.last < after.max)) + check_result({before=before, after=after}, pcall(ok, before.last * 2 > after.max)) + -- In this case, garbase collecting is not needed. + check_result({before=before, after=after}, pcall(eq, after.last, after.max)) + end) + + --[[ + 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. + ]]-- + it('function capture lvars', function() + if isasan() then + pending('ASAN build is difficult to estimate memory usage') + end + if iswin() and eval("executable('wmic')") == 0 then + pending('missing "wmic" command') + elseif eval("executable('ps')") == 0 then + pending('missing "ps" command') + end + + local pid = eval('getpid()') + local before = monitor_memory_usage(pid) + local fname = source([[ + if !exists('s:defined_func') + func s:f() + let x = l: + endfunc + endif + let s:defined_func = 1 + for _ in range(10000) + call s:f() + endfor + ]]) + local after = monitor_memory_usage(pid) + for _ = 1, 3 do + feed_command('so '..fname) + end + local last = monitor_memory_usage(pid) + check_result({before=before, after=after, last=last}, + pcall(ok, before.last < last.last)) + check_result({before=before, after=after, last=last}, + pcall(ok, last.last < after.max + (after.last - before.last))) + end) +end) |