Mercurial > vim
diff src/userfunc.c @ 16003:879829e44091 v8.1.1007
patch 8.1.1007: using closure may consume a lot of memory
commit https://github.com/vim/vim/commit/209b8e3e3bf7a4a3d102134124120f6c7f57d560
Author: Bram Moolenaar <Bram@vim.org>
Date: Thu Mar 14 13:43:24 2019 +0100
patch 8.1.1007: using closure may consume a lot of memory
Problem: Using closure may consume a lot of memory.
Solution: unreference items that are no longer needed. Add a test. (Ozaki
Kiichi, closes #3961)
author | Bram Moolenaar <Bram@vim.org> |
---|---|
date | Thu, 14 Mar 2019 13:45:06 +0100 |
parents | bd75c9df2a14 |
children | 3d6b282e2d6e |
line wrap: on
line diff
--- a/src/userfunc.c +++ b/src/userfunc.c @@ -39,12 +39,12 @@ static hashtab_T func_hashtab; /* 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; +// 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,97 +586,129 @@ add_nr_var( } /* - * Free "fc" and what it contains. + * Free "fc". */ - static void -free_funccal( - funccall_T *fc, - int free_val) /* a: vars were allocated */ + static void +free_funccal(funccall_T *fc) { - listitem_T *li; - int i; + 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. */ + 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); } /* + * 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); +} + +/* * 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) { + int may_free_fc = fc->fc_refcount <= 0; + int 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->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); - } + // 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 { - 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. */ + 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; - v = HI2DI(hi); - copy_tv(&v->di_tv, &v->di_tv); + di = HI2DI(hi); + copy_tv(&di->di_tv, &di->di_tv); } } - - /* Make a copy of the a:000 items, since we didn't do that above. */ + } + + 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 (++made_copy == 10000) + } + + 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. This can happen when - // repetitively calling a function that creates a reference to + // 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,7 +763,7 @@ call_user_func( line_breakcheck(); /* check for CTRL-C hit */ - fc = (funccall_T *)alloc(sizeof(funccall_T)); + fc = (funccall_T *)alloc_clear(sizeof(funccall_T)); if (fc == NULL) return; fc->caller = current_funccal; @@ -832,16 +864,15 @@ call_user_func( { v = &fc->fixvar[fixvar_idx++].var; v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX; + STRCPY(v->di_key, name); } else { - v = (dictitem_T *)alloc((unsigned)(sizeof(dictitem_T) - + STRLEN(name))); + v = dictitem_alloc(name); if (v == NULL) break; - v->di_flags = DI_FLAGS_RO | DI_FLAGS_FIX | DI_FLAGS_ALLOC; + v->di_flags |= DI_FLAGS_RO | DI_FLAGS_FIX; } - STRCPY(v->di_key, name); /* Note: the values are copied directly to avoid alloc/free. * "argvars" must have VAR_FIXED for v_lock. */ @@ -860,9 +891,11 @@ call_user_func( 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; + 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,7 +1121,7 @@ funccal_unref(funccall_T *fc, ufunc_T *f if (fc == *pfc) { *pfc = fc->caller; - free_funccal(fc, TRUE); + free_funccal_contents(fc); return; } } @@ -3646,7 +3679,7 @@ free_unref_funccal(int copyID, int testi { fc = *pfc; *pfc = fc->caller; - free_funccal(fc, TRUE); + free_funccal_contents(fc); did_free = TRUE; did_free_funccal = TRUE; }