From a8ecc1ae6d82d80f63ed75ac5c882520843905ae Mon Sep 17 00:00:00 2001 From: bfredl Date: Thu, 23 Jun 2022 19:19:20 +0200 Subject: perf(map): avoid extraneous heap allocations when setting mappings - don't immediately vim_strsave and then xfree a heap buffer. - allow replace_termcodes to take in a buffer instead of allocating it - grug! memory allocation bad! --- src/nvim/keycodes.c | 16 ++++++-- src/nvim/mapping.c | 105 +++++++++++++++++++++++++++++----------------------- src/nvim/menu.c | 1 + src/nvim/option.c | 1 + 4 files changed, 74 insertions(+), 49 deletions(-) (limited to 'src') diff --git a/src/nvim/keycodes.c b/src/nvim/keycodes.c index e0ada32ec2..dd78ebe722 100644 --- a/src/nvim/keycodes.c +++ b/src/nvim/keycodes.c @@ -861,6 +861,9 @@ int get_mouse_button(int code, bool *is_click, bool *is_drag) /// @param[in] from What characters to replace. /// @param[in] from_len Length of the "from" argument. /// @param[out] bufp Location where results were saved in case of success (allocated). +/// if *bufp is non-NULL, it will be used directly. it is +/// assumed to be 128 bytes long (enough for transcoding LHS +/// of mapping) /// Will be set to NULL in case of failure. /// @param[in] flags REPTERM_FROM_PART see above /// REPTERM_DO_LT also translate @@ -885,10 +888,12 @@ char *replace_termcodes(const char *const from, const size_t from_len, char **co const bool do_backslash = !(cpo_flags & FLAG_CPO_BSLASH); // backslash is a special character const bool do_special = !(flags & REPTERM_NO_SPECIAL); + bool allocated = (*bufp == NULL); + // Allocate space for the translation. Worst case a single character is // replaced by 6 bytes (shifted special key), plus a NUL at the end. - const size_t buf_len = from_len * 6 + 1; - result = xmalloc(buf_len); + const size_t buf_len = allocated ? from_len * 6 + 1 : 128; + result = allocated ? xmalloc(buf_len) : *bufp; src = (char_u *)from; @@ -907,6 +912,9 @@ char *replace_termcodes(const char *const from, const size_t from_len, char **co // Copy each byte from *from to result[dlen] while (src <= end) { + if (!allocated && dlen + 64 > buf_len) { + return NULL; + } // Check for special <> keycodes, like "" if (do_special && ((flags & REPTERM_DO_LT) || ((end - src) >= 3 && STRNCMP(src, "", 4) != 0))) { @@ -1000,7 +1008,9 @@ char *replace_termcodes(const char *const from, const size_t from_len, char **co } result[dlen] = NUL; - *bufp = xrealloc(result, dlen + 1); + if (allocated) { + *bufp = xrealloc(result, dlen + 1); + } return *bufp; } diff --git a/src/nvim/mapping.c b/src/nvim/mapping.c index 34bdd9785a..13df2ebea1 100644 --- a/src/nvim/mapping.c +++ b/src/nvim/mapping.c @@ -104,10 +104,10 @@ static void mapblock_free(mapblock_T **mpp) xfree(mp->m_keys); if (!mp->m_simplified) { NLUA_CLEAR_REF(mp->m_luaref); + xfree(mp->m_str); + xfree(mp->m_orig_str); } - XFREE_CLEAR(mp->m_str); - XFREE_CLEAR(mp->m_orig_str); - XFREE_CLEAR(mp->m_desc); + xfree(mp->m_desc); *mpp = mp->m_next; xfree(mp); } @@ -254,14 +254,12 @@ static void showmap(mapblock_T *mp, bool local) /// @param[in] orig_rhs_len `strlen` of orig_rhs. /// @param[in] cpo_flags See param docs for @ref replace_termcodes. /// @param[out] mapargs MapArguments struct holding the replaced strings. -static void set_maparg_lhs_rhs(const char *const orig_lhs, const size_t orig_lhs_len, +static bool set_maparg_lhs_rhs(const char *const orig_lhs, const size_t orig_lhs_len, const char *const orig_rhs, const size_t orig_rhs_len, const LuaRef rhs_lua, const int cpo_flags, MapArguments *const mapargs) { - char *lhs_buf = NULL; - char *alt_lhs_buf = NULL; - char *rhs_buf = NULL; + char lhs_buf[128]; // If mapping has been given as ^V say, then replace the term codes // with the appropriate two bytes. If it is a shifted special key, unshift @@ -273,13 +271,20 @@ static void set_maparg_lhs_rhs(const char *const orig_lhs, const size_t orig_lhs // If something like is simplified to 0x08 then mark it as simplified. bool did_simplify = false; const int flags = REPTERM_FROM_PART | REPTERM_DO_LT; - char *replaced = replace_termcodes(orig_lhs, orig_lhs_len, &lhs_buf, flags, &did_simplify, + char *bufarg = lhs_buf; + char *replaced = replace_termcodes(orig_lhs, orig_lhs_len, &bufarg, flags, &did_simplify, cpo_flags); + if (replaced == NULL) { + return false; + } mapargs->lhs_len = STRLEN(replaced); STRLCPY(mapargs->lhs, replaced, sizeof(mapargs->lhs)); if (did_simplify) { - replaced = replace_termcodes(orig_lhs, orig_lhs_len, &alt_lhs_buf, flags | REPTERM_NO_SIMPLIFY, + replaced = replace_termcodes(orig_lhs, orig_lhs_len, &bufarg, flags | REPTERM_NO_SIMPLIFY, NULL, cpo_flags); + if (replaced == NULL) { + return false; + } mapargs->alt_lhs_len = STRLEN(replaced); STRLCPY(mapargs->alt_lhs, replaced, sizeof(mapargs->alt_lhs)); } else { @@ -298,14 +303,14 @@ static void set_maparg_lhs_rhs(const char *const orig_lhs, const size_t orig_lhs mapargs->rhs_len = 0; mapargs->rhs_is_noop = true; } else { + char *rhs_buf = NULL; replaced = replace_termcodes(orig_rhs, orig_rhs_len, &rhs_buf, REPTERM_DO_LT, NULL, cpo_flags); mapargs->rhs_len = STRLEN(replaced); // XXX: replace_termcodes may produce an empty string even if orig_rhs is non-empty // (e.g. a single ^V, see :h map-empty-rhs) mapargs->rhs_is_noop = orig_rhs_len != 0 && mapargs->rhs_len == 0; - mapargs->rhs = xcalloc(mapargs->rhs_len + 1, sizeof(char_u)); - STRLCPY(mapargs->rhs, replaced, mapargs->rhs_len + 1); + mapargs->rhs = (char_u *)replaced; } } else { char tmp_buf[64]; @@ -317,10 +322,7 @@ static void set_maparg_lhs_rhs(const char *const orig_lhs, const size_t orig_lhs (char_u)KS_EXTRA, KE_LUA, rhs_lua); mapargs->rhs = vim_strsave((char_u *)tmp_buf); } - - xfree(lhs_buf); - xfree(alt_lhs_buf); - xfree(rhs_buf); + return true; } /// Parse a string of |:map-arguments| into a @ref MapArguments struct. @@ -346,27 +348,26 @@ static int str_to_mapargs(const char_u *strargs, bool is_unmap, MapArguments *ma { const char_u *to_parse = strargs; to_parse = (char_u *)skipwhite((char *)to_parse); - MapArguments parsed_args; // copy these into mapargs "all at once" when done - memset(&parsed_args, 0, sizeof(parsed_args)); + memset(mapargs, 0, sizeof(*mapargs)); // Accept , , , ,