From a9f563ab62f6035729f79c3c701a5b219f1b982f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Linse?= Date: Wed, 18 Aug 2021 14:19:01 +0200 Subject: [PATCH 1/3] refactor(highlight): make syn_check_group alloc free for existing group --- src/nvim/buffer.c | 2 +- src/nvim/syntax.c | 51 +++++++++++++++++++++++------------------------ 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index 587ef74b35..29d4fc786a 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -4253,7 +4253,7 @@ int build_stl_str_hl( if (*fmt_p == '#') { stl_items[curitem].type = Highlight; stl_items[curitem].start = out_p; - stl_items[curitem].minwid = -syn_namen2id(t, (int)(fmt_p - t)); + stl_items[curitem].minwid = -syn_name2id_len(t, (size_t)(fmt_p - t)); curitem++; fmt_p++; } diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c index e9ee63970c..319966158a 100644 --- a/src/nvim/syntax.c +++ b/src/nvim/syntax.c @@ -3372,7 +3372,7 @@ static void syn_cmd_clear(exarg_T *eap, int syncing) XFREE_CLEAR(SYN_CLSTR(curwin->w_s)[scl_id].scl_list); } } else { - id = syn_namen2id(arg, (int)(arg_end - arg)); + id = syn_name2id_len(arg, (int)(arg_end - arg)); if (id == 0) { EMSG2(_(e_nogroup), arg); break; @@ -3542,7 +3542,7 @@ syn_cmd_list( else syn_list_cluster(id - SYNID_CLUSTER); } else { - int id = syn_namen2id(arg, (int)(arg_end - arg)); + int id = syn_name2id_len(arg, (int)(arg_end - arg)); if (id == 0) { EMSG2(_(e_nogroup), arg); } else { @@ -6635,7 +6635,7 @@ void do_highlight(const char *line, const bool forceit, const bool init) // ":highlight {group-name}": list highlighting for one group. if (!doclear && !dolink && ends_excmd((uint8_t)(*linep))) { - id = syn_namen2id((const char_u *)line, (int)(name_end - line)); + id = syn_name2id_len((const char_u *)line, (int)(name_end - line)); if (id == 0) { emsgf(_("E411: highlight group not found: %s"), line); } else { @@ -7470,21 +7470,34 @@ static void set_hl_attr(int idx) } } +int syn_name2id(const char_u *name) + FUNC_ATTR_NONNULL_ALL +{ + return syn_name2id_len(name, STRLEN(name)); + +} + /// Lookup a highlight group name and return its ID. /// /// @param highlight name e.g. 'Cursor', 'Normal' /// @return the highlight id, else 0 if \p name does not exist -int syn_name2id(const char_u *name) +int syn_name2id_len(const char_u *name, size_t len) FUNC_ATTR_NONNULL_ALL { int i; - char_u name_u[200]; + char_u name_u[201]; - /* Avoid using stricmp() too much, it's slow on some systems */ - /* Avoid alloc()/free(), these are slow too. ID names over 200 chars - * don't deserve to be found! */ - STRLCPY(name_u, name, 200); + if (len == 0 || len > 200) { + return 0; + } + + // Avoid using stricmp() too much, it's slow on some systems */ + // Avoid alloc()/free(), these are slow too. ID names over 200 chars + // don't deserve to be found! + memcpy(name_u, name, len); + name_u[len] = '\0'; vim_strup(name_u); + // TODO(bfredl): what is a hash table? for (i = highlight_ga.ga_len; --i >= 0; ) if (HL_TABLE()[i].sg_name_u != NULL && STRCMP(name_u, HL_TABLE()[i].sg_name_u) == 0) @@ -7524,17 +7537,6 @@ char_u *syn_id2name(int id) return HL_TABLE()[id - 1].sg_name; } -/* - * Like syn_name2id(), but take a pointer + length argument. - */ -int syn_namen2id(const char_u *linep, int len) -{ - char_u *name = vim_strnsave(linep, len); - int id = syn_name2id(name); - xfree(name); - - return id; -} /// Find highlight group name in the table and return its ID. /// If it doesn't exist yet, a new entry is created. @@ -7543,14 +7545,11 @@ int syn_namen2id(const char_u *linep, int len) /// @param len length of \p pp /// /// @return 0 for failure else the id of the group -int syn_check_group(const char_u *pp, int len) +int syn_check_group(const char_u *name, int len) { - char_u *name = vim_strnsave(pp, len); - int id = syn_name2id(name); + int id = syn_name2id_len(name, len); if (id == 0) { // doesn't exist yet - id = syn_add_group(name); - } else { - xfree(name); + return syn_add_group(vim_strnsave(name, len)); } return id; } From fca52f5f32844d8820e6a1ef2678bc79c5e6d8d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Linse?= Date: Wed, 18 Aug 2021 14:29:42 +0200 Subject: [PATCH 2/3] feat(match): allow hl group to be defined after :match command --- runtime/doc/vim_diff.txt | 2 ++ src/nvim/testdir/test_match.vim | 5 ++++- src/nvim/window.c | 3 +-- test/functional/eval/match_functions_spec.lua | 15 +++++++-------- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/runtime/doc/vim_diff.txt b/runtime/doc/vim_diff.txt index 7e1bd3e087..79fb996bc1 100644 --- a/runtime/doc/vim_diff.txt +++ b/runtime/doc/vim_diff.txt @@ -159,6 +159,7 @@ Commands: |:drop| is always available |:Man| is available by default, with many improvements such as completion |:sign-define| accepts a `numhl` argument, to highlight the line number + |:match| can be invoked before highlight group is defined Events: |Signal| @@ -176,6 +177,7 @@ Functions: |msgpackdump()|, |msgpackparse()| provide msgpack de/serialization |stdpath()| |system()|, |systemlist()| can run {cmd} directly (without 'shell') + |matchadd()| can be called before highlight group is defined Highlight groups: |highlight-blend| controls blend level for a highlight group diff --git a/src/nvim/testdir/test_match.vim b/src/nvim/testdir/test_match.vim index 09448ca71b..0fd76a23ea 100644 --- a/src/nvim/testdir/test_match.vim +++ b/src/nvim/testdir/test_match.vim @@ -157,7 +157,10 @@ func Test_match_error() endfunc func Test_matchadd_error() - call assert_fails("call matchadd('GroupDoesNotExist', 'X')", 'E28:') + call clearmatches() + " Nvim: not an error anymore: + call matchadd('GroupDoesNotExist', 'X') + call assert_equal([{'group': 'GroupDoesNotExist', 'pattern': 'X', 'priority': 10, 'id': 13}], getmatches()) call assert_fails("call matchadd('Search', '\\(')", 'E475:') call assert_fails("call matchadd('Search', 'XXX', 1, 123, 1)", 'E715:') call assert_fails("call matchadd('Error', 'XXX', 1, 3)", 'E798:') diff --git a/src/nvim/window.c b/src/nvim/window.c index 5677a4d39d..fefbab822e 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -6534,8 +6534,7 @@ int match_add(win_T *wp, const char *const grp, const char *const pat, cur = cur->next; } } - if ((hlg_id = syn_name2id((const char_u *)grp)) == 0) { - EMSG2(_(e_nogroup), grp); + if ((hlg_id = syn_check_group((const char_u *)grp, strlen(grp))) == 0) { return -1; } if (pat != NULL && (regprog = vim_regcomp((char_u *)pat, RE_MAGIC)) == NULL) { diff --git a/test/functional/eval/match_functions_spec.lua b/test/functional/eval/match_functions_spec.lua index f399ef47d3..9f168c913a 100644 --- a/test/functional/eval/match_functions_spec.lua +++ b/test/functional/eval/match_functions_spec.lua @@ -6,7 +6,6 @@ local clear = helpers.clear local funcs = helpers.funcs local command = helpers.command local exc_exec = helpers.exc_exec -local pcall_err = helpers.pcall_err before_each(clear) @@ -40,13 +39,13 @@ describe('setmatches()', function() }}, funcs.getmatches()) end) - it('fails with -1 if highlight group is not defined', function() - eq('Vim:E28: No such highlight group name: 1', - pcall_err(funcs.setmatches, {{group=1, pattern=2, id=3, priority=4}})) - eq({}, funcs.getmatches()) - eq('Vim:E28: No such highlight group name: 1', - pcall_err(funcs.setmatches, {{group=1, pos1={2}, pos2={6}, id=3, priority=4, conceal=5}})) - eq({}, funcs.getmatches()) + it('does not fail if highlight group is not defined', function() + eq(0, funcs.setmatches{{group=1, pattern=2, id=3, priority=4}}) + eq({{group='1', pattern='2', id=3, priority=4}}, + funcs.getmatches()) + eq(0, funcs.setmatches{{group=1, pos1={2}, pos2={6}, id=3, priority=4, conceal=5}}) + eq({{group='1', pos1={2}, pos2={6}, id=3, priority=4, conceal='5'}}, + funcs.getmatches()) end) end) From bb4b4d79a8dd0eb60aa37f0b889558c4ae8e9317 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Linse?= Date: Wed, 18 Aug 2021 17:05:28 +0200 Subject: [PATCH 3/3] perf(highlight): use a hashtable for highlight group names syn_name2id and syn_check_group go brr. Note: this has impact mostly when using multiple filetypes, as the old syn_name2id was optimized to return latest added groups quickly (which will be the latest filetype) --- src/nvim/main.c | 1 + src/nvim/map.c | 1 + src/nvim/map.h | 1 + src/nvim/syntax.c | 47 ++++++++++++++++++++++++++++------------------- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/nvim/main.c b/src/nvim/main.c index 2cdf01b146..136608afdf 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -166,6 +166,7 @@ void early_init(mparm_T *paramp) init_path(argv0 ? argv0 : "nvim"); init_normal_cmds(); // Init the table of Normal mode commands. highlight_init(); + syntax_init(); #ifdef WIN32 OSVERSIONINFO ovi; diff --git a/src/nvim/map.c b/src/nvim/map.c index 7d97b7f13d..86dc257e40 100644 --- a/src/nvim/map.c +++ b/src/nvim/map.c @@ -194,6 +194,7 @@ static inline bool ColorKey_eq(ColorKey ae1, ColorKey ae2) MAP_IMPL(int, int, DEFAULT_INITIALIZER) MAP_IMPL(cstr_t, ptr_t, DEFAULT_INITIALIZER) +MAP_IMPL(cstr_t, int, DEFAULT_INITIALIZER) MAP_IMPL(ptr_t, ptr_t, DEFAULT_INITIALIZER) MAP_IMPL(uint64_t, ptr_t, DEFAULT_INITIALIZER) MAP_IMPL(uint64_t, ssize_t, SSIZE_INITIALIZER) diff --git a/src/nvim/map.h b/src/nvim/map.h index 7bd3d31330..a35a2c1672 100644 --- a/src/nvim/map.h +++ b/src/nvim/map.h @@ -36,6 +36,7 @@ // MAP_DECLS(int, int) MAP_DECLS(cstr_t, ptr_t) +MAP_DECLS(cstr_t, int) MAP_DECLS(ptr_t, ptr_t) MAP_DECLS(uint64_t, ptr_t) MAP_DECLS(uint64_t, ssize_t) diff --git a/src/nvim/syntax.c b/src/nvim/syntax.c index 319966158a..1d45df8ebd 100644 --- a/src/nvim/syntax.c +++ b/src/nvim/syntax.c @@ -53,9 +53,9 @@ static bool did_syntax_onoff = false; /// Structure that stores information about a highlight group. /// The ID of a highlight group is also called group ID. It is the index in /// the highlight_ga array PLUS ONE. -struct hl_group { +typedef struct hl_group { char_u *sg_name; ///< highlight group name - char_u *sg_name_u; ///< uppercase of sg_name + char *sg_name_u; ///< uppercase of sg_name bool sg_cleared; ///< "hi clear" was used int sg_attr; ///< Screen attr @see ATTR_ENTRY int sg_link; ///< link to this highlight group ID @@ -80,7 +80,7 @@ struct hl_group { char *sg_rgb_sp_name; ///< RGB special color name int sg_blend; ///< blend level (0-100 inclusive), -1 if unset -}; +} HlGroup; /// \addtogroup SG_SET /// @{ @@ -91,6 +91,7 @@ struct hl_group { // builtin |highlight-groups| static garray_T highlight_ga = GA_EMPTY_INIT_VALUE; +Map(cstr_t, int) *highlight_unames; static inline struct hl_group * HL_TABLE(void) { @@ -384,6 +385,11 @@ static int current_line_id = 0; // unique number for current line static int syn_time_on = FALSE; # define IF_SYN_TIME(p) (p) +void syntax_init(void) +{ + highlight_unames = map_new(cstr_t, int)(); +} + // Set the timeout used for syntax highlighting. // Use NULL to reset, no timeout. void syn_set_timeout(proftime_T *tm) @@ -7113,6 +7119,7 @@ void free_highlight(void) xfree(HL_TABLE()[i].sg_name_u); } ga_clear(&highlight_ga); + map_free(cstr_t, int)(highlight_unames); } #endif @@ -7474,7 +7481,6 @@ int syn_name2id(const char_u *name) FUNC_ATTR_NONNULL_ALL { return syn_name2id_len(name, STRLEN(name)); - } /// Lookup a highlight group name and return its ID. @@ -7484,25 +7490,22 @@ int syn_name2id(const char_u *name) int syn_name2id_len(const char_u *name, size_t len) FUNC_ATTR_NONNULL_ALL { - int i; - char_u name_u[201]; + char name_u[201]; if (len == 0 || len > 200) { + // ID names over 200 chars don't deserve to be found! return 0; } // Avoid using stricmp() too much, it's slow on some systems */ - // Avoid alloc()/free(), these are slow too. ID names over 200 chars - // don't deserve to be found! + // Avoid alloc()/free(), these are slow too. memcpy(name_u, name, len); name_u[len] = '\0'; - vim_strup(name_u); - // TODO(bfredl): what is a hash table? - for (i = highlight_ga.ga_len; --i >= 0; ) - if (HL_TABLE()[i].sg_name_u != NULL - && STRCMP(name_u, HL_TABLE()[i].sg_name_u) == 0) - break; - return i + 1; + vim_strup((char_u *)name_u); + + // map_get(..., int) returns 0 when no key is present, which is + // the expected value for missing highlight group. + return map_get(cstr_t, int)(highlight_unames, name_u); } /// Lookup a highlight group name and return its attributes. @@ -7592,7 +7595,7 @@ static int syn_add_group(char_u *name) return 0; } - char_u *const name_up = vim_strsave_up(name); + char *const name_up = (char *)vim_strsave_up(name); // Append another syntax_highlight entry. struct hl_group* hlgp = GA_APPEND_VIA_PTR(struct hl_group, &highlight_ga); @@ -7604,7 +7607,11 @@ static int syn_add_group(char_u *name) hlgp->sg_blend = -1; hlgp->sg_name_u = name_up; - return highlight_ga.ga_len; /* ID is index plus one */ + int id = highlight_ga.ga_len; // ID is index plus one + + map_put(cstr_t, int)(highlight_unames, name_up, id); + + return id; } /// When, just after calling syn_add_group(), an error is discovered, this @@ -7612,8 +7619,10 @@ static int syn_add_group(char_u *name) static void syn_unadd_group(void) { highlight_ga.ga_len--; - xfree(HL_TABLE()[highlight_ga.ga_len].sg_name); - xfree(HL_TABLE()[highlight_ga.ga_len].sg_name_u); + HlGroup *item = &HL_TABLE()[highlight_ga.ga_len]; + map_del(cstr_t, int)(highlight_unames, item->sg_name_u); + xfree(item->sg_name); + xfree(item->sg_name_u); }