vimPlugins.nvim-treesitter: collate grammars

[stephen-huan]

Description of changes

Currently pkgs.vimPlugins.nvim-treesitter.withAllGrammars adds 279 directories to 'runtimepath', significantly slowing down startup time. See :help nvim-treesitter-performance.

nvim-treesitter checks the 'runtimepath' on startup in order to discover
available parsers and queries and index them. As a consequence, a very long
'runtimepath' might result in delayed startup times.

This PR changes the directory structure from vimplugin-treesitter-${grammar}/parser/${name}.so for each grammar to a single directory of the form vimplugin-treesitter-grammars/parser/*.so containing all specified grammars.

In my configuration, this saves about 100 ms. I use vim.loader.enable() and the contents of test.py are

print("hello")

• pkgs.vimPlugins.nvim-treesitter.withPlugins (_: [ ]) (0 additional grammars)
  • nvim --startuptime time.txt -> ~110 ms
  • nvim --startuptime time.txt test.py -> ~200 ms
• with these 56 treesitter grammars
  • nvim --startuptime time.txt -> ~130 ms
  • nvim --startuptime time.txt test.py -> ~220 ms
• pkgs.vimPlugins.nvim-treesitter.withAllGrammars (279 grammars)
  • nvim --startuptime time.txt -> ~200 ms
  • nvim --startuptime time.txt test.py -> ~300 ms
• after collating (this PR)
  • with 56 treesitter grammars
    • nvim --startuptime time.txt -> ~115 ms
    • nvim --startuptime time.txt test.py -> ~200 ms
  • pkgs.vimPlugins.nvim-treesitter.withAllGrammars
    • nvim --startuptime time.txt -> ~115 ms
    • nvim --startuptime time.txt test.py -> ~200 ms

We can conclude the cost of adding each grammar to 'runtimepath' is linear and roughly 0.36 ms per grammar. After collating, the startup cost is negligible even with all grammars added.

Ran nix build .#vimPlugins.nvim-treesitter.tests.check-queries and nixpkgs-review rev HEAD.

Also (a variant of this PR in my personal configuration) passes :checkhealth.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[r-vdp]

LGTM! I rebuild my system with this patch and it seems to work correctly so far.

[r-vdp]

I wonder what's the trade-off between copying the files into a new store path (less individual runtime dependencies, duplication of files in the store) versus symlinking from the new store path to the existing ones (avoids duplication of files in the store, keeps all individual store paths as runtime dependencies).

The startup time should be the same in both cases, I guess, assuming that resolving the symlinks is very cheap.

[stephen-huan]

I wonder what's the trade-off between copying the files into a new store path (less individual runtime dependencies, duplication of files in the store) versus symlinking from the new store path to the existing ones (avoids duplication of files in the store, keeps all individual store paths as runtime dependencies).

It wasn't my intention to change this, good catch. Considering withAllGrammars is ~190 MB I think we should avoid duplication (although this sort of thing should be optimized away by nix store optimise?).

4.3M   ┌── ocaml_interface.so│█                                          │   2%
4.8M   ├── tlaplus.so        │██                                         │   3%
4.8M   ├── ocaml.so          │██                                         │   3%
5.0M   ├── slang.so          │██                                         │   3%
5.4M   ├── c_sharp.so        │██                                         │   3%
5.4M   ├── julia.so          │██                                         │   3%
7.3M   ├── objc.so           │██                                         │   4%
8.7M   ├── nim.so            │██                                         │   5%
 11M   ├── gnuplot.so        │███                                        │   6%
 17M   ├── verilog.so        │████                                       │   9%
188M ┌─┴ parser              │██████████████████████████████████████████ │ 100%

The startup time should be the same in both cases, I guess, assuming that resolving the symlinks is very cheap.

There's no measurable performance impact, as far as I can tell.

I've changed cp to ln -s for consistency with grammarToPlugin.

nixpkgs/pkgs/applications/editors/neovim/utils.nix

Lines 221 to 230 in 2b32fd6

	toVimPlugin (runCommand "treesitter-grammar-${name}"
	{
	meta = {
	platforms = lib.platforms.all;
	} // grammar.meta;
	}
	''
	mkdir -p $out/parser
	ln -s ${grammar}/parser $out/parser/${name}.so
	'');

[PerchunPak]

@stephen-huan if there are 0 rebuilds, how does it change anything?

[stephen-huan]

@PerchunPak it changes the directory structure which causes rebuilds. I don't know how ofborg determines the number of rebuilds, but it isn't accurate here.

[PerchunPak]

@stephen-huan nixpkgs-review also doesn't detect any rebuilds, which means Nix doesn't detect any rebuilds

I also manually rebuilt vimPlugins.nvim-treesitter.withAllGrammars and don't see any difference in generated file structure:

~/dev/nixpkgs @67602af1d270 ❯ git checkout upstream/master
~/dev/nixpkgs @67602af1d270 ❯ nix build .#vimPlugins.nvim-treesitter.withAllGrammars --rebuild
~/dev/nixpkgs @67602af1d270 ❯ tree result/ | sha256sum
555a37a8ccf7bd18ff6fccbc148256b8579a865c66edd1eeb147f8ebc420d5db  -
~/dev/nixpkgs @67602af1d270 ❯ rm result
~/dev/nixpkgs @67602af1d270 ❯ git cherry-pick 54f33c58c45e4677699162dbd864adfd363377e1
[detached HEAD 6a9bc3fa9706] vimPlugins.nvim-treesitter: collate grammars
 Author: Stephen Huan <[email protected]>
 Date: Wed Jun 12 01:43:57 2024 -0400
 1 file changed, 16 insertions(+), 2 deletions(-)
~/dev/nixpkgs @6a9bc3fa9706 ❯ nix build .#vimPlugins.nvim-treesitter.withAllGrammars --rebuild
~/dev/nixpkgs @6a9bc3fa9706 ❯ tree result/ | sha256sum
555a37a8ccf7bd18ff6fccbc148256b8579a865c66edd1eeb147f8ebc420d5db  -

[stephen-huan]

@PerchunPak The outer derivation is the same since the plugin is the same. Instead, compare pkgs.vimPlugins.nvim-treesitter.withAllGrammars.passthru.dependencies before and after the patch (before, there are ~287 derivations, after, there is only one). This makes a difference when the plugin is added to programs.neovim.plugins, for example.

See my personal configuration. You can check that it's different when built end-to-end by nvim $(which nvim), finding the set packpath^=/nix/store/<hash>-vim-pack-dir line, and checking the /nix/store/<hash>-vim-pack-dir/pack/myNeovimPackages/start/ directory (one will have ~287 vimplugin-treesitter-${grammar} directories, the other a single vimplugin-treesitter-grammars directory). This change in structure reduces the number of entries on runtimepath which results in the observable timing differences profiled above.

See also the package tests nix build .#vimPlugins.nvim-treesitter.tests.check-queries.

nixpkgs/pkgs/applications/editors/vim/plugins/nvim-treesitter/overrides.nix

Lines 70 to 93 in 54f33c5

	tests.check-queries =
	let
	nvimWithAllGrammars = neovim.override {
	configure.packages.all.start = [ withAllGrammars ];
	};
	in
	runCommand "nvim-treesitter-check-queries"
	{
	nativeBuildInputs = [ nvimWithAllGrammars ];
	CI = true;
	}
	''
	touch $out
	export HOME=$(mktemp -d)
	ln -s ${withAllGrammars}/CONTRIBUTING.md .
	nvim --headless "+luafile ${withAllGrammars}/scripts/check-queries.lua" | tee log
	if grep -q Warning log; then
	echo "Error: warnings were emitted by the check"
	exit 1
	fi
	'';
	};

[PerchunPak]

True, sorry for my confusion and laziness in reading the code. If you use passthru, that explains why there are no rebuilds (passthru doesn't affect the out hash by design).

By just rebuilding my system using this PR, my startup time decreased from 100ms to 70-80ms.

LGTM! CC @teto

[stephen-huan]

@PerchunPak a bit late, but thanks for the review and no worries about the confusion!

[obreitwi]

Unfortunately, this change causes my neovim to not detect any grammars. Both my treesitter configuration, but also nvim-treesitter.withAllGrammars do not work anymore.

By that I mean that :checkhealth nvim-treesitter does not detect any grammars being installed. Simply reverting the PR resolves the issue for me.

Once I have time, I will investigate further.

[PerchunPak]

Hey @obreitwi!

Could you try building from master? #339589 should fix some things. If it doesn't work, please open a new issue and ping me there

[obreitwi]

Hey @PerchunPak, unfortunately, using <nixpkgs@master>.vimPlugins.nvim-treesitter.withAllGrammars as part of home-manager's programs.neovim.plugins does not resolve the issue.

I am currently on the move with limited time, so I will probably only get around to investigate properly and open an issue next week.

[figsoda]

I don't think it's possible to merge all the parsers into one plugin unless we change how vim plugin dependencies work in nixpkgs. Here is the reasoning behind when I initially split the different grammars into separate plugins: #201548, and for the same reason this PR breaks my (and probably @obreitwi 's as well) nvim-treesitter.withAllGrammars setup.

[PerchunPak]

Welcome back!

Yeah, you are right, this PR caused many headaches for me, so I am giving up and just opening a revert PR