Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor performance with Treesitter highlighting on deep if-else in c #24965

Closed
kaddkaka opened this issue Aug 31, 2023 · 15 comments · Fixed by #28512
Closed

Poor performance with Treesitter highlighting on deep if-else in c #24965

kaddkaka opened this issue Aug 31, 2023 · 15 comments · Fixed by #28512
Labels
performance issues reporting performance problems treesitter

Comments

@kaddkaka
Copy link
Contributor

kaddkaka commented Aug 31, 2023

Problem

Editing or moving close to the last line of a very long if-elseif-elseif-else structure takes about ~1 second per action to perform.

Steps to reproduce

Open a file with the content below. Move to the bottom with G. notice how it takes a long time.

$ /usr/local/bin/nvim --version
NVIM v0.10.0-dev-994+g670c7609c-dirty
Build type: Debug
LuaJIT 2.1.1693350652
Run "nvim -V1 -v" for more info

$ /usr/local/bin/nvim -u NORC -c 'lua vim.treesitter.start()' projects/tmp/treesitter/test_perf.c

File content:

#include "cstdio"


int main () {
        if (1 == 0) {
                printf("3");
        } else if (4 == 1) {
                printf("5");
        } else if (7 == 1) {
                printf("7");
        } else if (10 == 1) {
                printf("9");
        } else if (13 == 1) {
        } else if (15 == 1) {
                printf("12");
                printf("13");
                printf("14");
                printf("15");
                printf("16");
                printf("17");
                printf("18");
                printf("19");
                printf("20");
                printf("21");
        } else if (27 == 1) {
                printf("23");
        } else if (30 == 1) {
                printf("25");
        } else if (33 == 1) {
                printf("27");
        } else if (36 == 1) {
                printf("29");
        } else if (39 == 1) {
                printf("31");
        } else if (42 == 1) {
                printf("33");
        } else if (45 == 1) {
                printf("35");
        } else if (48 == 1) {
                printf("37");
        } else if (51 == 1) {
                printf("39");
        } else if (54 == 1) {
                printf("41");
        } else if (57 == 1) {
                printf("43");
        } else if (60 == 1) {
                printf("45");
        } else if (63 == 1) {
                printf("47");
        } else if (66 == 1) {
                printf("49");
        } else if (69 == 1) {
                printf("51");
        } else if (72 == 1) {
                printf("53");
        } else if (75 == 1) {
                printf("55");
        } else if (78 == 1) {
                printf("57");
        } else if (81 == 1) {
                printf("59");
        } else if (84 == 1) {
                printf("61");
        } else if (87 == 1) {
                printf("63");
        } else if (90 == 1) {
                printf("65");
        } else if (93 == 1) {
                printf("67");
        } else if (96 == 1) {
                printf("69");
        } else if (99 == 1) {
        } else if (101 == 1) {
                printf("72");
        } else if (104 == 1) {
                printf("74");
        } else if (107 == 1) {
                printf("76");
        } else if (110 == 1) {
                printf("78");
        } else if (113 == 1) {
                printf("80");
        } else if (116 == 1) {
                printf("82");
        } else if (119 == 1) {
                printf("84");
        } else if (122 == 1) {
                printf("86");
        } else if (125 == 1) {
                printf("88");
        } else if (128 == 1) {
        } else if (130 == 1) {
                printf("91");
        } else if (133 == 1) {
                printf("93");
        } else if (136 == 1) {
                printf("95");
        } else if (139 == 1) {
                printf("97");
        } else if (142 == 1) {
                printf("99");
        } else if (145 == 1) {
                printf("101");
        } else if (148 == 1) {
                printf("103");
        } else if (151 == 1) {
                printf("105");
        } else if (154 == 1) {
                printf("107");
        } else if (157 == 1) {
        } else if (159 == 1) {
                printf("110");
        } else if (162 == 1) {
                printf("112");
        } else if (165 == 1) {
                printf("114");
        } else if (168 == 1) {
                printf("116");
        } else if (171 == 1) {
                printf("118");
        } else if (174 == 1) {
                printf("120");
        } else if (177 == 1) {
                printf("122");
        } else if (180 == 1) {
                printf("124");
        } else if (183 == 1) {
                printf("126");
        } else if (186 == 1) {
                printf("128");
        } else if (189 == 1) {
                printf("130");
        } else if (192 == 1) {
                printf("132");
        } else if (195 == 1) {
                printf("134");
        } else if (198 == 1) {
                printf("136");
        } else if (201 == 1) {
                printf("138");
        } else if (204 == 1) {
                printf("140");
        } else if (207 == 1) {
                printf("142");
        } else if (210 == 1) {
                printf("144");
        } else if (213 == 1) {
                printf("146");
        } else if (216 == 1) {
                printf("148");
        } else if (219 == 1) {
                printf("150");
        } else if (222 == 1) {
                printf("152");
        } else if (225 == 1) {
                printf("154");
        } else if (228 == 1) {
                printf("156");
        } else if (231 == 1) {
                printf("158");
        } else if (234 == 1) {
                printf("160");
        } else if (237 == 1) {
                printf("162");
        } else if (240 == 1) {
                printf("164");
        } else if (243 == 1) {
                printf("166");
        } else if (246 == 1) {
                printf("168");
        } else if (249 == 1) {
                printf("170");
        } else if (252 == 1) {
                printf("172");
        } else if (255 == 1) {
                printf("174");
        } else if (258 == 1) {
                printf("176");
        } else if (261 == 1) {
                printf("178");
        } else if (264 == 1) {
                printf("180");
        } else if (267 == 1) {
                printf("182");
        } else if (270 == 1) {
                printf("184");
        } else if (273 == 1) {
                printf("186");
        } else if (276 == 1) {
                printf("188");
        } else if (279 == 1) {
                printf("190");
        } else if (282 == 1) {
                printf("192");
        } else if (285 == 1) {
                printf("194");
        } else if (288 == 1) {
                printf("196");
        } else if (291 == 1) {
                printf("198");
        } else if (294 == 1) {
                printf("200");
        } else if (297 == 1) {
                printf("202");
        } else if (300 == 1) {
                printf("204");
        } else if (303 == 1) {
                printf("206");
        } else if (306 == 1) {
                printf("208");
        } else if (308 == 1) {
                printf("210");
        } else if (310 == 1) {
                printf("212");
        } else if (312 == 1) {
                printf("214");
        } else if (314 == 1) {
                printf("216");
        } else if (316 == 1) {
                printf("218");
        } else if (318 == 1) {
                printf("220");
        } else if (320 == 1) {
                printf("222");
        } else if (322 == 1) {
                printf("224");
        } else if (324 == 1) {
                printf("226");
        } else if (326 == 1) {
                printf("228");
        } else if (328 == 1) {
                printf("230");
        } else if (330 == 1) {
                printf("232");
        } else if (332 == 1) {
                printf("234");
        } else if (334 == 1) {
                printf("236");
        } else if (336 == 1) {
                printf("238");
        } else if (338 == 1) {
                printf("240");
        } else if (340 == 1) {
                printf("242");
        } else if (342 == 1) {
                printf("244");
        } else if (344 == 1) {
                printf("246");
        } else if (346 == 1) {
                printf("248");
        } else if (348 == 1) {
                printf("250");
        } else if (350 == 1) {
                printf("252");
        } else if (352 == 1) {
                printf("254");
        } else if (354 == 1) {
                printf("256");
        } else if (356 == 1) {
                printf("258");
        } else if (358 == 1) {
                printf("260");
        } else if (360 == 1) {
                printf("262");
        } else if (362 == 1) {
                printf("264");
        } else if (364 == 1) {
                printf("266");
        } else if (366 == 1) {
                printf("268");
        } else if (368 == 1) {
                printf("270");
        } else if (370 == 1) {
                printf("272");
        } else if (372 == 1) {
                printf("274");
        } else if (374 == 1) {
                printf("276");
        } else if (376 == 1) {
                printf("278");
        } else if (378 == 1) {
                printf("280");
        } else if (380 == 1) {
                printf("282");
        } else if (382 == 1) {
                printf("284");
        } else if (384 == 1) {
                printf("286");
        } else if (386 == 1) {
                printf("288");
        } else if (388 == 1) {
                printf("290");
        } else if (390 == 1) {
                printf("292");
        } else if (392 == 1) {
                printf("294");
        } else if (394 == 1) {
                printf("296");
        } else if (396 == 1) {
                printf("298");
        } else if (398 == 1) {
                printf("300");
        } else if (400 == 1) {
                printf("302");
        } else if (402 == 1) {
                printf("304");
        } else if (404 == 1) {
                printf("306");
        } else if (406 == 1) {
                printf("308");
        } else if (408 == 1) {
                printf("310");
        } else if (410 == 1) {
                printf("312");
        } else if (412 == 1) {
                printf("314");
        } else if (414 == 1) {
                printf("316");
        } else if (416 == 1) {
                printf("318");
        } else if (418 == 1) {
                printf("320");
        } else if (420 == 1) {
                printf("322");
        } else if (422 == 1) {
                printf("324");
        } else if (424 == 1) {
                printf("326");
        } else if (426 == 1) {
                printf("328");
        } else if (428 == 1) {
                printf("330");
        } else if (430 == 1) {
                printf("332");
        } else if (432 == 1) {
                printf("334");
        } else if (434 == 1) {
                printf("336");
        } else if (436 == 1) {
                printf("338");
        } else if (438 == 1) {
                printf("340");
        } else if (440 == 1) {
                printf("342");
        } else if (442 == 1) {
                printf("344");
        } else if (444 == 1) {
                printf("346");
        } else if (446 == 1) {
                printf("348");
        } else if (448 == 1) {
                printf("350");
        } else if (450 == 1) {
                printf("352");
        } else if (452 == 1) {
                printf("354");
        } else if (454 == 1) {
                printf("356");
        } else if (456 == 1) {
                printf("358");
        } else if (458 == 1) {
                printf("360");
        } else if (460 == 1) {
                printf("362");
        } else if (462 == 1) {
                printf("364");
        } else if (464 == 1) {
                printf("366");
        } else if (466 == 1) {
                printf("368");
        } else if (468 == 1) {
                printf("370");
        } else if (470 == 1) {
                printf("372");
        } else if (472 == 1) {
                printf("374");
        } else if (474 == 1) {
                printf("376");
        } else if (476 == 1) {
                printf("378");
        } else if (478 == 1) {
                printf("380");
        } else if (480 == 1) {
                printf("382");
        } else if (482 == 1) {
                printf("384");
        } else if (484 == 1) {
                printf("386");
        } else if (486 == 1) {
                printf("388");
        } else if (488 == 1) {
                printf("390");
        } else if (490 == 1) {
                printf("392");
        } else if (492 == 1) {
                printf("394");
        } else if (494 == 1) {
                printf("396");
        } else if (496 == 1) {
                printf("398");
        } else if (498 == 1) {
                printf("400");
        } else if (500 == 1) {
                printf("402");
        } else if (502 == 1) {
                printf("404");
        } else if (504 == 1) {
                printf("406");
        } else if (506 == 1) {
                printf("408");
        } else if (508 == 1) {
                printf("410");
        } else if (510 == 1) {
                printf("412");
        } else if (512 == 1) {
                printf("414");
        } else if (514 == 1) {
                printf("416");
        } else if (516 == 1) {
                printf("418");
        } else if (518 == 1) {
                printf("420");
        } else if (520 == 1) {
                printf("422");
        } else if (522 == 1) {
                printf("424");
        } else if (524 == 1) {
                printf("426");
        } else if (526 == 1) {
                printf("428");
        } else if (528 == 1) {
                printf("430");
        } else if (530 == 1) {
                printf("432");
        } else if (532 == 1) {
                printf("434");
        } else if (534 == 1) {
                printf("436");
        } else if (536 == 1) {
                printf("438");
        } else if (538 == 1) {
                printf("440");
        } else if (540 == 1) {
                printf("442");
        } else if (542 == 1) {
                printf("444");
        } else if (544 == 1) {
                printf("446");
        } else if (546 == 1) {
                printf("448");
        } else if (548 == 1) {
                printf("450");
        } else if (550 == 1) {
                printf("452");
        } else if (552 == 1) {
                printf("454");
        } else if (554 == 1) {
                printf("456");
        } else if (556 == 1) {
                printf("458");
        } else if (558 == 1) {
                printf("460");
        } else if (560 == 1) {
                printf("462");
        } else if (562 == 1) {
                printf("464");
        } else if (564 == 1) {
                printf("466");
        } else if (566 == 1) {
                printf("468");
        } else if (568 == 1) {
                printf("470");
        } else if (570 == 1) {
                printf("472");
        } else if (572 == 1) {
                printf("474");
        } else if (574 == 1) {
                printf("476");
        } else if (576 == 1) {
                printf("478");
        } else if (578 == 1) {
                printf("480");
        } else if (580 == 1) {
                printf("482");
        } else if (582 == 1) {
                printf("484");
        } else if (584 == 1) {
                printf("486");
        } else if (586 == 1) {
                printf("488");
        } else if (588 == 1) {
                printf("490");
        } else if (590 == 1) {
                printf("492");
        } else if (592 == 1) {
                printf("494");
        } else if (594 == 1) {
                printf("496");
        } else if (596 == 1) {
                printf("498");
        } else if (598 == 1) {
                printf("500");
        } else if (600 == 1) {
                printf("502");
        } else if (602 == 1) {
                printf("504");
        } else if (604 == 1) {
                printf("506");
        } else if (606 == 1) {
                printf("508");
        } else if (608 == 1) {
                printf("510");
        } else if (610 == 1) {
                printf("512");
        } else if (612 == 1) {
                printf("514");
        } else if (614 == 1) {
                printf("516");
        } else if (616 == 1) {
                printf("518");
        } else if (618 == 1) {
                printf("520");
        } else if (620 == 1) {
                printf("522");
        } else if (622 == 1) {
                printf("524");
        } else if (624 == 1) {
                printf("526");
        } else if (626 == 1) {
                printf("528");
        } else if (628 == 1) {
                printf("530");
        } else if (630 == 1) {
                printf("532");
        } else if (632 == 1) {
                printf("534");
        } else if (634 == 1) {
                printf("536");
        } else if (636 == 1) {
                printf("538");
        } else if (638 == 1) {
                printf("540");
        } else if (640 == 1) {
                printf("542");
        } else if (642 == 1) {
                printf("544");
        } else if (644 == 1) {
                printf("546");
        } else if (646 == 1) {
                printf("548");
        } else if (648 == 1) {
                printf("550");
        } else if (650 == 1) {
                printf("552");
        } else if (652 == 1) {
                printf("554");
        } else if (654 == 1) {
                printf("556");
        } else if (656 == 1) {
                printf("558");
        } else if (658 == 1) {
                printf("560");
        } else if (660 == 1) {
                printf("562");
        } else if (662 == 1) {
                printf("564");
        } else if (664 == 1) {
                printf("566");
        } else if (666 == 1) {
                printf("568");
        } else if (668 == 1) {
                printf("570");
        } else if (670 == 1) {
                printf("572");
        } else if (672 == 1) {
                printf("574");
        } else if (674 == 1) {
                printf("576");
        } else if (676 == 1) {
                printf("578");
        } else if (678 == 1) {
                printf("580");
        } else if (680 == 1) {
                printf("582");
        } else if (682 == 1) {
                printf("584");
        } else if (684 == 1) {
                printf("586");
        } else if (686 == 1) {
                printf("588");
        } else if (688 == 1) {
                printf("590");
        } else if (690 == 1) {
                printf("592");
        } else if (692 == 1) {
                printf("594");
        } else if (694 == 1) {
                printf("596");
        } else if (696 == 1) {
                printf("598");
        } else if (698 == 1) {
                printf("600");
        } else if (700 == 1) {
                printf("602");
        } else if (702 == 1) {
                printf("604");
        } else if (704 == 1) {
                printf("606");
        } else if (706 == 1) {
                printf("608");
        } else if (708 == 1) {
                printf("610");
        } else if (710 == 1) {
                printf("612");
        } else if (712 == 1) {
                printf("614");
        } else if (714 == 1) {
                printf("616");
        } else if (716 == 1) {
                printf("618");
        } else if (718 == 1) {
                printf("620");
        } else if (720 == 1) {
                printf("622");
        } else if (722 == 1) {
                printf("624");
        } else if (724 == 1) {
                printf("626");
        } else if (726 == 1) {
                printf("628");
        } else if (728 == 1) {
                printf("630");
        } else if (730 == 1) {
                printf("632");
        } else if (732 == 1) {
                printf("634");
        } else if (734 == 1) {
                printf("636");
        } else if (736 == 1) {
                printf("638");
        } else if (738 == 1) {
                printf("640");
        } else if (740 == 1) {
                printf("642");
        } else if (742 == 1) {
                printf("644");
        } else if (744 == 1) {
                printf("646");
        } else if (746 == 1) {
                printf("648");
	}
}

Expected behavior

Fast and rapid movement and editing

Neovim version (nvim -v)

NVIM v0.10.0-dev-994+g670c7609c-dirty

Vim (not Nvim) behaves the same?

no

Operating system/version

Ubuntu 22.04

Terminal name/version

wezterm version: 20230820-104238-6c7aa815

$TERM environment variable

xterm-256color

Installation

build from repo

@kaddkaka kaddkaka added the bug issues reporting wrong behavior label Aug 31, 2023
@kaddkaka kaddkaka changed the title Poor performance with Treesitter highlighting on deep if-else in cpp Poor performance with Treesitter highlighting on deep if-else in c Aug 31, 2023
@clason

This comment was marked as off-topic.

@kaddkaka

This comment was marked as off-topic.

@ObserverOfTime

This comment was marked as off-topic.

@clason clason added performance issues reporting performance problems and removed bug issues reporting wrong behavior labels Sep 2, 2023
@tomtomjhj
Copy link
Sponsor Contributor

Most time is spent processing has-ancestor? predicate.

patch for profiling
diff --git a/runtime/lua/vim/treesitter/highlighter.lua b/runtime/lua/vim/treesitter/highlighter.lua
index 496193c6e..a24d022d3 100644
--- a/runtime/lua/vim/treesitter/highlighter.lua
+++ b/runtime/lua/vim/treesitter/highlighter.lua
@@ -315,6 +315,7 @@ function TSHighlighter._on_spell_nav(_, _, buf, srow, _, erow, _)
   end
 end
 
+local start
 ---@private
 ---@param _win integer
 ---@param buf integer
@@ -335,6 +336,8 @@ api.nvim_set_decoration_provider(ns, {
   on_win = TSHighlighter._on_win,
   on_line = TSHighlighter._on_line,
   _on_spell_nav = TSHighlighter._on_spell_nav,
+  on_start = function() start = vim.uv.hrtime() end,
+  on_end = function() print('redrawn', (vim.uv.hrtime() - start) / 1000000, vim.treesitter.query.times()) end
 })
 
 return TSHighlighter
diff --git a/runtime/lua/vim/treesitter/query.lua b/runtime/lua/vim/treesitter/query.lua
index 8cbbffcd6..0f0a04a00 100644
--- a/runtime/lua/vim/treesitter/query.lua
+++ b/runtime/lua/vim/treesitter/query.lua
@@ -407,6 +407,29 @@ local predicate_handlers = {
 -- As we provide lua-match? also expose vim-match?
 predicate_handlers['vim-match?'] = predicate_handlers['match?']
 
+local times = {} ---@type table<string, number>
+for k, f in pairs(predicate_handlers) do
+  times[k] = 0
+  predicate_handlers[k] = function(...)
+    local start = vim.uv.hrtime()
+    local ret = { f(...) }
+    times[k] = times[k] + (vim.uv.hrtime() - start) / 1000000
+    return unpack(ret)
+  end
+end
+
+function M.times()
+  local all_zeros = true
+  local result = vim.inspect(times)
+  for k, _ in pairs(times) do
+    if times[k] > 0 then all_zeros = false end
+    times[k] = 0
+  end
+  if not all_zeros then
+    return result
+  end
+end
+
 ---@class TSMetadata
 ---@field range? Range
 ---@field conceal? string

time spent while redrawing (in ms)

redrawn 365.606942 {
  ["any-of?"] = 0.131375,
  ["contains?"] = 0,
  ["eq?"] = 0,
  ["has-ancestor?"] = 362.042793,
  ["has-parent?"] = 0,
  ["lua-match?"] = 0.168706,
  ["match?"] = 0,
  ["vim-match?"] = 0
}

The treesitter c grammar parses else if as if_statement nested in else_clause, and has-ancestor? has time complexity linear in the tree depth.

while node do
if ancestor_types[node:type()] then
return true
end
node = node:parent()
end

I think there should be some limit on the distance or time.

@clason
Copy link
Member

clason commented Sep 28, 2023

Yes, #has-ancestor? is known to be a performance sink (I think I documented that somewhere, but it may not have made it into the official docs.)

A distance limit is unacceptable, since some functionality (other than highlighting) may rely on a correct global parse. A general time limit is already planned and should cover this as well: #22420

@tomtomjhj
Copy link
Sponsor Contributor

A general time limit is already planned and should cover this as well: #22420

I think that patch in the current form doesn't fix this issue. Those predicates are called during on_line_impl, so it won't be affected by the parsing timeout.

87%  handler
  <- 100%  match_preds < iter < fn < for_each_tree < on_line_impl < highlighter.lua:292

It wouldn't too difficult to add the timeout logic to treessiter highlighter's decoration provider, but I think per-query distance limit (e.g., adding "limit" parameter to has-ancestor?) is better in some aspects.

  • Aborting one query does not affect other queries, while global timeout would abort all the remaining queries, resulting in worse highlights.
  • Query authors can set a good limit based on the knowledge of the grammar. If accuracy is important, they can opt out of the limit.

@clason
Copy link
Member

clason commented Sep 28, 2023

e.g., adding "limit" parameter to has-ancestor?

Concretely, how would that look for the C queries here? (It's always better to design an API around concrete use cases instead of generic hypotheticals ;))

@tomtomjhj
Copy link
Sponsor Contributor

The problematic query for the OP's example is this

((call_expression
function: (identifier) @function.builtin)
(#has-ancestor? @function.builtin attribute_specifier))

I was thinking of something like this

-  (#has-ancestor? @function.builtin attribute_specifier))
+  (#has-ancestor-within? 20 @function.builtin attribute_specifier))

Since #has-ancestor? takes multiple candidates for the ancestor, we need a variant that takes the limit value (or use #set!).

@clason
Copy link
Member

clason commented Sep 28, 2023

Yes, but what concretely would be the limit value? Is this something fixed by the actual query, or just some sort of "I don't expect that most cases will need more than 20..."

(There's a big difference between an objectively correct value and a fudge factor that needs to be tuned.)

Since #has-ancestor? takes multiple candidates for the ancestor, we need a variant that takes the limit value (or use #set!).

Adding yet another variant is a hard "no" from me, I'm afraid. If we cannot support this with the current predicate, #set! is the correct approach. (If we even need multiple candidates; I can't find an example in nvim-treesitter queries.)

If #has-parent? can then be simply replaced by #has-ancestor? 1 -- without a performance price -- that would significantly strengthen the case here, too.

@tomtomjhj
Copy link
Sponsor Contributor

Yes, but what concretely would be the limit value? Is this something fixed by the actual query, or just some sort of "I don't expect that most cases will need more than 20..."

(There's a big difference between an objectively correct value and a fudge factor that needs to be tuned.)

For that example, I meant the latter. For something objectively correct, we would need an interface that takes a general predicate on the path to ancestor. This is relatively simple in some cases.

  • Example 1. The c query mentioned above is designed to highlight function call-like attributes, e.g., aligned in the following code:

    struct X {
        int a __attribute__((aligned(4)));
    } __attribute__((aligned(16)));

    From the specification of the attribute syntax, it seems that the search should stop on encountering compound_statement node (the list of statements wrapped by {/}). We can use a metadata attribute to specify such behavior:

    (#set! "search-break" "compound_statement")
    
  • Example 2. This cpp query highlights namespaced function name in a function call (e.g. f in a::b::c::d::e::f()). To limit the search to qualified_identifier nodes, we can introduce another attribute:

    ((qualified_identifier
      (identifier) @function.call) @_parent
     (#has-ancestor? @_parent call_expression)
     (#set! "search-only" "qualified_identifier"))
    

I don't know if such "per-node" predicates are sufficient for other cases.

But in any case, I think distance limit is good enough even if it can't be totally correct since the worst thing that could happen is just some hightlight going slightly off.

Adding yet another variant is a hard "no" from me, I'm afraid. If we cannot support this with the current predicate, #set! is the correct approach. (If we even need multiple candidates; I can't find an example in nvim-treesitter queries.)

Or we can make has-ancestor always take the limit value and use 0 to denote unlimited search.

If #has-parent? can then be simply replaced by #has-ancestor? 1 -- without a performance price -- that would significantly strengthen the case here, too.

I think that should be fine. In fact that's how #has-parent? is implemented in nvim-treesitter. https://github.com/nvim-treesitter/nvim-treesitter/blob/cc2f94ed1dfa008c23e16bbd17f56b967ceb6740/lua/nvim-treesitter/query_predicates.lua#L96

@clason
Copy link
Member

clason commented Oct 1, 2023

For that example, I meant the latter.

In which case, wouldn't it be better to just handle this in the predicate handler itself? If we only want to guard against unbounded recursion, we can treat this as a global limit (possibly tunable, similarly to max_start_depth).

@tomtomjhj
Copy link
Sponsor Contributor

In which case, wouldn't it be better to just handle this in the predicate handler itself? If we only want to guard against unbounded recursion, we can treat this as a global limit (possibly tunable, similarly to max_start_depth).

Sounds good. I'll try it later when I have time.

@vanaigr
Copy link
Contributor

vanaigr commented Mar 24, 2024

I made a prototype that decreases the time to

redrawn 22.87975 {
  ["any-contains?"] = 0,
  ["any-eq?"] = 0,
  ["any-lua-match?"] = 0,
  ["any-match?"] = 0,
  ["any-of?"] = 0.064202,
  ["any-vim-match?"] = 0,
  ["contains?"] = 0,
  ["eq?"] = 0,
  ["has-ancestor?"] = 14.913001,
  ["has-parent?"] = 0,
  ["lua-match?"] = 0.634667,
  ["match?"] = 0,
  ["vim-match?"] = 0
}

from

redrawn 429.159457 {
  ["any-contains?"] = 0,
  ["any-eq?"] = 0,
  ["any-lua-match?"] = 0,
  ["any-match?"] = 0,
  ["any-of?"] = 0.283093,
  ["any-vim-match?"] = 0,
  ["contains?"] = 0,
  ["eq?"] = 0,
  ["has-ancestor?"] = 421.619772,
  ["has-parent?"] = 0,
  ["lua-match?"] = 0.48094,
  ["match?"] = 0,
  ["vim-match?"] = 0
}

by adding a function to tree-sitter.

has-ancestor? is slow because treesitter's function for getting the node's parent has to traverse all parents of the given node (in reverse order) to find the closest one. Since this function is called for every parent, this results in has-ancestor? being for the number of parents.

This can be decreased to n if treesitter exposes a function to get node's parents in reverse order.

@clason
Copy link
Member

clason commented Mar 24, 2024

Very nice! Would you care making a PR to tree-sitter?

@vanaigr
Copy link
Contributor

vanaigr commented Mar 24, 2024

Very nice! Would you care making a PR to tree-sitter?

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance issues reporting performance problems treesitter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants