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

feat(treesitter): add support for wasm parsers #28415

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Conversation

lewis6991
Copy link
Member

@lewis6991 lewis6991 commented Apr 19, 2024

Problem

Treesitter parsers are currently distributed as compiled shared objects which need to be compiled by the users (with the help of nvim-treesitter).

Solution

Allow loading wasm built parsers.

Notes:

  • For now this is an optional build-time feature (via ENABLE_WASMTIME) which will be dynamically linked.
    • Builds with make CMAKE_EXTRA_FLAGS=-DENABLE_WASMTIME=ON DEPS_CMAKE_FLAGS=-DENABLE_WASMTIME=ON
  • Treesitter requires being built with TREE_SITTER_FEATURE_WASM and also requires wasmtime at build time. So no [uv_]dlopen 😢
  • wasmtime has includes that require C11 (static_assert).
  • To reduce re-compilation times, install sccache and build with RUSTC_WRAPPER=sccache make ....

Status

Waiting for the next tree-sitter release with the commit we're pulling in here to allow us to build against a wasmtime release with the build changes required to avoid unacceptable regressions on macOS. (Ideally, that release would also be built against the same version we use, but that is blocked by the same build system changes; luckily so far the versions have not become incompatible for our purpose.)

@lewis6991 lewis6991 force-pushed the tswasm branch 2 times, most recently from 7264715 to be7cc1c Compare April 19, 2024 15:19
@lewis6991 lewis6991 added DO NOT MERGE Nothing to see here; move along needs:discussion issue needs attention from an expert, or PR proposes significant changes to architecture or API labels Apr 19, 2024
@clason clason added this to the 0.11 milestone Apr 19, 2024
@lewis6991 lewis6991 force-pushed the tswasm branch 5 times, most recently from c2519a9 to 31f9b03 Compare April 20, 2024 17:27
@lewis6991 lewis6991 force-pushed the tswasm branch 2 times, most recently from 7b627ac to ca9e93d Compare April 22, 2024 07:55
@lewis6991 lewis6991 removed the DO NOT MERGE Nothing to see here; move along label Apr 22, 2024
cmake/Deps.cmake Outdated Show resolved Hide resolved
cmake/FindWasmtime.cmake Outdated Show resolved Hide resolved
cmake.deps/deps.txt Outdated Show resolved Hide resolved
@lewis6991 lewis6991 force-pushed the tswasm branch 3 times, most recently from 9f77fb6 to 2ecad21 Compare April 22, 2024 08:51
@lewis6991 lewis6991 marked this pull request as ready for review April 22, 2024 11:36
@github-actions github-actions bot requested review from bfredl and clason April 22, 2024 11:41
@clason
Copy link
Member

clason commented Apr 22, 2024

Is this something we want to add to BUILD.md (yet)?

@clason clason force-pushed the tswasm branch 2 times, most recently from cbfa704 to 5ccf96b Compare July 8, 2024 12:14
@neovim neovim deleted a comment from lewis6991 Jul 8, 2024
@clason
Copy link
Member

clason commented Jul 8, 2024

Note: Investigate how wasm parsers and native parsers for the same language interact (i.e., trying to (un)load a wasm parser after loading a native or vice versa).

@clason clason force-pushed the tswasm branch 3 times, most recently from 2d414a3 to 3c30933 Compare July 22, 2024 16:36
@clason
Copy link
Member

clason commented Jul 24, 2024

Note 2: Look at using sccache instead of ccache to speed up (re)building.

EDIT Seems to make a noticeable difference: cold build from git clean -fdx went from ~120s (with ccache) to ~85s (with sccache, see below). Baseline on this machine is ~30s for master with ccache. You still have to pay the cost of the full LTO, which is the biggest item. Specifically the wasmtime build:

  • before: Finished fastest-runtime profile [optimized] target(s) in 1m 02s
  • after: Finished fastest-runtime profile [optimized] target(s) in 38.99s

Tested with

time RUSTC_WRAPPER=/opt/homebrew/bin/sccache CMAKE_C_COMPILER_LAUNCHER=sccache CMAKE_EXTRA_FLAGS=-DENABLE_WASMTIME=ON DEPS_CMAKE_FLAGS=-DENABLE_WASMTIME=ON make

@dundargoc

This comment was marked as resolved.

@lewis6991

This comment was marked as resolved.

@clason clason force-pushed the tswasm branch 3 times, most recently from 6c00a4c to 8799870 Compare August 22, 2024 15:35
Problem: Installing treesitter parser is hard (harder than
climbing to heaven).

Solution: Add optional support for wasm parsers with `wasmtime`.

Notes:

* Needs to be enabled by setting `ENABLE_WASMTIME` for tree-sitter and
  Neovim. Build with
  `make CMAKE_EXTRA_FLAGS=-DENABLE_WASMTIME=ON
  DEPS_CMAKE_FLAGS=-DENABLE_WASMTIME=ON`
* Adds optional Rust (obviously) and C11 dependencies.
* Wasmtime comes with a lot of features that can negatively affect
  Neovim performance due to library and symbol table size. Make sure to
  build with minimal features and full LTO.
* To reduce re-compilation times, install `sccache` and build with
  `RUSTC_WRAPPER=<path/to/sccache> make ...`
@clason clason marked this pull request as ready for review August 26, 2024 14:23
@clason clason enabled auto-merge (rebase) August 26, 2024 14:23
@clason clason merged commit 688b961 into neovim:master Aug 26, 2024
44 of 45 checks passed
@justinmk
Copy link
Member

justinmk commented Aug 27, 2024

trying to (un)load a wasm parser after loading a native or vice versa).

Does that mean it's possible to actually unload parsers now? I thought that wasn't really supported yet.

will there be support for downloading these parsers by the neovim

see #22313

@ofseed

This comment has been minimized.

@neovim neovim locked and limited conversation to collaborators Aug 31, 2024
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ci:skip-news dependencies build dependencies (LuaJIT, LibUV, etc.) treesitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants