From 0df681a91d6b86395609e6fc40efb4d8623d72c9 Mon Sep 17 00:00:00 2001 From: bfredl Date: Mon, 29 Apr 2024 14:12:39 +0200 Subject: [PATCH] fix(treesitter): make tests for memoize more robust Instead of painfully messing with timing to determine if queries were reparsed, we can simply keep a counter next to the call to ts_query_new Also memoization had a hidden dependency on the garbage collection of the the key, a hash value which never is kept around in memory. this was done intentionally as the hash does not capture all relevant state for the query (external included files) even if actual query objects still would be reachable in memory. To make the test fully deterministic in CI, we explicitly control GC. --- src/nvim/api/vim.c | 4 +++- src/nvim/lua/treesitter.c | 1 + src/nvim/lua/treesitter.h | 5 +++++ src/nvim/main.c | 1 + test/functional/treesitter/query_spec.lua | 20 ++++++++------------ 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/nvim/api/vim.c b/src/nvim/api/vim.c index d62ea46e1b..e4e4ae29f2 100644 --- a/src/nvim/api/vim.c +++ b/src/nvim/api/vim.c @@ -45,6 +45,7 @@ #include "nvim/keycodes.h" #include "nvim/log.h" #include "nvim/lua/executor.h" +#include "nvim/lua/treesitter.h" #include "nvim/macros_defs.h" #include "nvim/mapping.h" #include "nvim/mark.h" @@ -1806,12 +1807,13 @@ Float nvim__id_float(Float flt) /// @return Map of various internal stats. Dictionary nvim__stats(Arena *arena) { - Dictionary rv = arena_dict(arena, 5); + Dictionary rv = arena_dict(arena, 6); PUT_C(rv, "fsync", INTEGER_OBJ(g_stats.fsync)); PUT_C(rv, "log_skip", INTEGER_OBJ(g_stats.log_skip)); PUT_C(rv, "lua_refcount", INTEGER_OBJ(nlua_get_global_ref_count())); PUT_C(rv, "redraw", INTEGER_OBJ(g_stats.redraw)); PUT_C(rv, "arena_alloc_count", INTEGER_OBJ((Integer)arena_alloc_count)); + PUT_C(rv, "ts_query_parse_count", INTEGER_OBJ((Integer)tslua_query_parse_count)); return rv; } diff --git a/src/nvim/lua/treesitter.c b/src/nvim/lua/treesitter.c index f4bc454790..8befc6d32d 100644 --- a/src/nvim/lua/treesitter.c +++ b/src/nvim/lua/treesitter.c @@ -1318,6 +1318,7 @@ int tslua_parse_query(lua_State *L) size_t len; const char *src = lua_tolstring(L, 2, &len); + tslua_query_parse_count++; uint32_t error_offset; TSQueryError error_type; TSQuery *query = ts_query_new(lang, src, (uint32_t)len, &error_offset, &error_type); diff --git a/src/nvim/lua/treesitter.h b/src/nvim/lua/treesitter.h index 4ef9a10602..14df06e184 100644 --- a/src/nvim/lua/treesitter.h +++ b/src/nvim/lua/treesitter.h @@ -1,7 +1,12 @@ #pragma once #include // IWYU pragma: keep +#include + +#include "nvim/macros_defs.h" #ifdef INCLUDE_GENERATED_DECLARATIONS # include "lua/treesitter.h.generated.h" #endif + +EXTERN uint64_t tslua_query_parse_count INIT( = 0); diff --git a/src/nvim/main.c b/src/nvim/main.c index e2a3d32984..30b6b6e86b 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -63,6 +63,7 @@ #include "nvim/log.h" #include "nvim/lua/executor.h" #include "nvim/lua/secure.h" +#include "nvim/lua/treesitter.h" #include "nvim/macros_defs.h" #include "nvim/main.h" #include "nvim/mark.h" diff --git a/test/functional/treesitter/query_spec.lua b/test/functional/treesitter/query_spec.lua index 3dad61d79e..96665ee2e7 100644 --- a/test/functional/treesitter/query_spec.lua +++ b/test/functional/treesitter/query_spec.lua @@ -7,7 +7,6 @@ local eq = t.eq local insert = n.insert local exec_lua = n.exec_lua local pcall_err = t.pcall_err -local is_os = t.is_os local api = n.api local fn = n.fn @@ -72,11 +71,14 @@ void ui_refresh(void) return exec_lua( [[ local query, n = ... - local before = vim.uv.hrtime() + local before = vim.api.nvim__stats().ts_query_parse_count + collectgarbage("stop") for i=1, n, 1 do cquery = vim.treesitter.query.parse("c", ...) end - local after = vim.uv.hrtime() + collectgarbage("restart") + collectgarbage("collect") + local after = vim.api.nvim__stats().ts_query_parse_count return after - before ]], long_query, @@ -84,15 +86,9 @@ void ui_refresh(void) ) end - local firstrun = q(1) - local manyruns = q(100) - - -- First run should be at least 200x slower than an 100 subsequent runs. - local factor = is_os('win') and 100 or 200 - assert( - factor * manyruns < firstrun, - ('firstrun: %f ms, manyruns: %f ms'):format(firstrun / 1e6, manyruns / 1e6) - ) + eq(1, q(1)) + -- cache is cleared by garbage collection even if valid "cquery" reference is kept around + eq(1, q(100)) end) it('supports query and iter by capture (iter_captures)', function()