diff options
| -rw-r--r-- | docs/lsp-evaluation.md | 103 | ||||
| -rw-r--r-- | src/pardes.zig | 7 | ||||
| -rw-r--r-- | test/lspbench.zig | 13 | ||||
| -rw-r--r-- | test/lspfixture/broken.zig | 25 |
4 files changed, 142 insertions, 6 deletions
diff --git a/docs/lsp-evaluation.md b/docs/lsp-evaluation.md new file mode 100644 index 00000000..ae822213 --- /dev/null +++ b/docs/lsp-evaluation.md @@ -0,0 +1,103 @@ +# Three language backends, measured + +Same core, same seam (`src/lsp.zig`), same 17-probe harness (`zig build lspbench`) +over the same corpus. Three jj workspaces, three independent implementations, +one function each. + +| | **A · stdlib** | **B · client** | **C · in-process** | +|---|---|---|---| +| workspace | `pardes-ws-ast` | `pardes-ws-proto` | `pardes-ws-inproc` | +| what it is | `std.zig.Ast` + `AstGen`, hand-rolled scope walk | `zls` as a child process, JSON-RPC over a socketpair | ZLS linked as a Zig module, analyser called directly | +| **probes correct** | 12 / 17 | **17 / 17** | **17 / 17** | +| **false claims** | 0 | 0 | 0 | +| **implementation LOC** | 622 (+307 test) | 721 | **540** (+29 build) | +| new dependency | **none** | a `zls` binary at runtime | ZLS source (path dep) | +| binary size vs base | +6 MiB | +14 MiB | +35 MiB | +| memory | 3.7 MiB | 1.1 MiB **+ 37 MiB in the child** | 5.5 MiB | +| processes | **1** | 2 | **1** | + +### Latency (warm, µs unless noted) + +| operation | A · stdlib | B · client | C · in-process | +|---|---|---|---| +| goto, same file | **22** | 59 | 127 | +| goto, into `std` | 1 433 | **154** | 2 873 | +| goto, 5 000-line file | 2 699 | 182 | 4 181 | +| hover | **24** | 49 | 125 | +| document symbols | **28** | 196 | 89 | +| references | 6 624 | **65** | 168 | +| workspace symbols | 15 042 | **985** | 6 746 | +| diagnostics | 37 | **9** | 78 | +| **first query of the session** | 32 µs | **33 ms** | 4.4 ms | + +## What the numbers say + +**A is fastest at the thing you do most and slowest at everything that needs an +index.** A same-file `gd` in 22 µs is below the frame budget by three orders of +magnitude, and cold *is* warm because there is no index to build — but +references (6.6 ms) and workspace symbols (15 ms) are linear walks that will +grow with the tree. It answers 12 of 17 kinds and is honest about the other +five: field access, method calls and generics need a type resolver, and it is +not one. `gy`/`gi`/`=`/`SPC a`/`SPC r` stay inert rather than guessing. + +**B is the only one with a real index, and the only one that generalizes.** +References at 65 µs and workspace symbols at 985 µs are 100× and 15× the others +because zls maintains state across queries and pardes does not. It pays 33 ms +once per session for fork+exec+handshake, then 59–182 µs forever. Nothing in +`lsp_client.zig` knows Zig except a binary name and a `languageId` string — +pointing it at rust-analyzer or gopls is a table edit. Its costs are a second +process, 37 MiB of RSS that this harness does not charge it, and an external +binary that must exist and match the toolchain. + +**C is the smallest and the most complete.** 540 lines buy all 17 probes with no +subprocess, no JSON, no handshake and no version skew — the analyser is compiled +in. This is the "gut ZLS and call it directly" idea, and it works. Its latency is +middling and flat for the same reason A's is: it builds a `DocumentStore`, +answers, and throws it away, so there is no cache to be warm. A cross-query cache +is the obvious next step and is a real design (a global, a mutex, an invalidation +story), not a line of code. It costs +35 MiB of binary and couples pardes to a +ZLS checkout. + +## Verification notes + +Every number above was re-measured independently, not taken from the +implementers' reports. Three corrections came out of that: + +1. **The harness was wrong, and B caught it.** The diagnostics and format probes + originally pointed at clean, already-formatted source, where the correct + answer is nothing — indistinguishable from a backend that has no diagnostics. + That rewarded A and C for emitting a filler "no diagnostics" row and scored B + with 3 false claims for answering honestly. `test/lspfixture/broken.zig` now + gives those probes real work; B scores 17/17 on the corrected harness. +2. **A's unit tests never ran.** 23 tests compiled but were not collected — + `zig build unit-test` roots at `pardes.zig`, and Zig only collects tests from + files it actually analyzes, so the lazy `pub const lsp = @import(...)` reached + nothing. Once wired, 16 of 23 crashed on an invalid free in a test helper that + passed one arena as both the `gpa` and `arena` parameters. Fixed; 71/71 pass. +3. **C fuzzed itself and found two real crashes** (a decl's name token indexes + its own file, not the requesting one; and it is not necessarily an + `.identifier` on a half-typed line). Both fixed before reporting. + +All three keep the 56 existing snapshot scripts green, add a 57th driving the +real binary through the helix keymap, and pass `hxdiff` (360) and `hxparity` +(440) with no new waivers. + +## Recommendation + +**C (in-process), with B as the answer to a question pardes has not asked yet.** + +On the stated criteria — fewest lines, feature completeness, performance — C +wins: it is the smallest implementation, it answers everything, it needs no +second process, and its weakest number is a cache that does not exist rather +than a limit that cannot be lifted. + +The one thing that should override that: **pardes ships tree-sitter grammars for +25 languages.** C is Zig-only forever; B is the only implementation that will +ever answer `gd` in a Rust or Go buffer. If language intelligence is meant to +follow the syntax highlighting, B is the strategic choice and its 721 lines are +the cheapest multi-language client anyone will write. + +A is not the answer to "add LSP support", but it is a genuinely good answer to a +different question — 622 lines and zero dependencies for instant same-file +navigation and real semantic diagnostics. It is the one to keep if the ZLS +coupling in B and C ever becomes a problem. diff --git a/src/pardes.zig b/src/pardes.zig index 32de2f1b..682b0e3e 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -2935,8 +2935,11 @@ pub const Pardes = struct { const ln = std.mem.trimEnd(u8, rows, "\n"); var hi: usize = 0; while (hi < ln.len and look.isFileChar(ln[hi])) hi += 1; - // push the jumplist the way helix does before navigating away - pane.pinCursor(); + // exactly what a `/` result row does when n steps onto it: look the + // `path:LINE:COL` token from the pane that asked, so placement, + // dedup-onto-an-open-pane and centering are the ONE look path. + // (helix would also push its jumplist here; pardes has none, so + // there is nothing to push — do not read this as one.) return p.actOnSelection(.right, w.pane, ln[0..hi], null); } diff --git a/test/lspbench.zig b/test/lspbench.zig index 73b82e2b..307bdc49 100644 --- a/test/lspbench.zig +++ b/test/lspbench.zig @@ -72,18 +72,23 @@ const anchors = [_]Anchor{ .{ .file = "src/modal.zig", .needle = "pub fn ", .at = 7, .kind = .document_symbols, .expect = "" }, // references to a symbol used in several places .{ .file = "src/lsp.zig", .needle = "pub const Kind", .at = 11, .kind = .references, .expect = "" }, - // diagnostics on a file that should have none - .{ .file = "src/lsp.zig", .needle = "const std", .at = 6, .kind = .diagnostics, .expect = "" }, + // Diagnostics and format probe a DELIBERATELY BROKEN fixture, never the + // real source. On a clean corpus the correct answer is nothing, which is + // byte-identical to "this backend has no diagnostics" — a blind spot that + // rewards emitting a filler row and punishes honesty. test/lspfixture has + // an unused local (semantic: needs a compiler front end, not a tokenizer) + // and a misformatted fn (needs a formatter), so both probes have real work. + .{ .file = "test/lspfixture/broken.zig", .needle = "unused_local", .at = 0, .kind = .diagnostics, .expect = "broken.zig" }, // the remaining kinds, probed once each so the matrix is complete .{ .file = "src/lsp.zig", .needle = "lineCol(source", .at = 0, .kind = .declaration, .expect = "" }, .{ .file = "src/lsp.zig", .needle = "out: *std.ArrayList(u8)", .at = 10, .kind = .type_definition, .expect = "" }, .{ .file = "src/lsp.zig", .needle = "pub const Kind", .at = 11, .kind = .implementation, .expect = "" }, .{ .file = "src/lsp.zig", .needle = "pub const Kind", .at = 11, .kind = .select_refs, .expect = "" }, - .{ .file = "src/lsp.zig", .needle = "pub fn query", .at = 7, .kind = .format, .expect = "" }, + .{ .file = "test/lspfixture/broken.zig", .needle = "badly_spaced", .at = 0, .kind = .format, .expect = "" }, .{ .file = "src/lsp.zig", .needle = "pub fn query", .at = 7, .kind = .code_action, .expect = "" }, .{ .file = "src/lsp.zig", .needle = "pub fn query", .at = 7, .kind = .rename, .expect = "" }, .{ .file = "src/lsp.zig", .needle = "pub const Kind", .at = 11, .kind = .workspace_symbols, .expect = "" }, - .{ .file = "src/lsp.zig", .needle = "const std", .at = 6, .kind = .workspace_diagnostics, .expect = "" }, + .{ .file = "test/lspfixture/broken.zig", .needle = "unused_local", .at = 0, .kind = .workspace_diagnostics, .expect = "broken.zig" }, }; const Result = struct { diff --git a/test/lspfixture/broken.zig b/test/lspfixture/broken.zig new file mode 100644 index 00000000..22c86c9b --- /dev/null +++ b/test/lspfixture/broken.zig @@ -0,0 +1,25 @@ +//! A deliberately defective file, for the lspbench diagnostics and format +//! probes. It is NOT built and NOT imported by anything — `zig build` never +//! sees it, and the snapshot corpus never opens it. +//! +//! Why it exists: on a clean, already-formatted corpus the CORRECT answer to +//! "what is wrong with this file" is nothing, which the harness cannot tell +//! apart from a backend that has no diagnostics at all. That blind spot +//! rewards a backend for emitting a filler row and punishes one for being +//! honest, so the probes point here instead, where there is something real to +//! find: +//! +//! - `unused_local` is a SEMANTIC error. A tokenizer cannot see it; it takes +//! a compiler front end (AstGen/ZIR, or `zig ast-check`). +//! - the spacing in `badly_spaced` is a FORMATTING difference, which only a +//! formatter reports. +//! +//! Keep both. Removing either silently turns a probe back into a blind one. +pub fn hasUnusedLocal() u32 { + const unused_local = 41; + return 7; +} + +pub fn badly_spaced ( a : u32 , b : u32 ) u32 { + return a+b; +} |