Skip to content

chore(lsp): avoid redundant type checking#9337

Merged
asterite merged 6 commits intomasterfrom
ab/lsp-avoid-redundant-type-checking
Jul 28, 2025
Merged

chore(lsp): avoid redundant type checking#9337
asterite merged 6 commits intomasterfrom
ab/lsp-avoid-redundant-type-checking

Conversation

@asterite
Copy link
Copy Markdown
Collaborator

Description

Problem

No issue, just something I noticed.

Summary

The LSP communication has notifications (synchronous) and requests (asynchronous). Some of the notifications we are handling are:

  • a file was opened
  • a file contents changed
  • a file was saved
  • a file was closed

In each of these notifications we would run type-checking on the package the file is in. Because this is synchronous, all subsequent requests will have to wait for this notifications to be handled, meaning having to wait the type-checker.

The first thing this PR does is to not immediately type-check on notifications. Instead, we remember that the package changed and then, when needed, we type-check it. This already slightly improves things as we could sometimes get multiple "file contents changed" notifications in a row, if the user is typing relatively fast.

The next thing is that when the user opens a file in the editor we'd only need to type-check the package if it's the first time that package is opened. If the user opens another file in the same package there's no need to type-check things again. There was a noticeable slowdown when you'd navigate by clicking on items because if that item's file wasn't open you'd have to wait for type-checking to finish (and same if you'd click on another item next, etc.).

When a file is saved there's no need to type-check as "file saved" is only that (if you change a file and save it quickly we still get two separate notifications). What we do need to do is to output the errors to the "Problems" view. Then, there's a special handling here for when the user saves after no change happened. This is for when the user goes to the console and does git checkout some-other-branch where VSCode wouldn't realize files changed. In that case we do full type-checking (Rust Analyzer seems to do the same).

Then, when a file is closed there's no need to do type-checking.

Finally, we kept a counter of open files. When it reached zero we'd clean all cached data. That meant that if you closed all files and opened one, you'd have to wait for type-checking. I'm thinking keeping the cache is better here as, at least I, sometimes close all files when I have too many open and I don't want to lose all the type-checking information in that case.

Additional Context

This doesn't solve the issue of sometimes having to wait a couple of seconds before getting inlay hints, etc., but it should make things a bit more smooth/responsive.

Documentation

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: a1df3f9 Previous: e7fb7f3 Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob 3 s 2 s 1.50
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib 2 s 1 s 2
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_reset-kernel-lib 2 s 1 s 2
test_report_zkpassport_noir-ecdsa_ 2 s 1 s 2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@asterite asterite requested a review from a team July 28, 2025 13:02
@asterite asterite added this pull request to the merge queue Jul 28, 2025
Merged via the queue into master with commit 606bf9b Jul 28, 2025
101 checks passed
@asterite asterite deleted the ab/lsp-avoid-redundant-type-checking branch July 28, 2025 18:31
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Aug 4, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: forbid self-referencing type aliases
(noir-lang/noir#9103)
chore: add a mem2reg test for when all references need to be invalidated
(noir-lang/noir#9377)
fix(ssa): Do not check ArrayGet/Set as unreachable for Brillig
(noir-lang/noir#9376)
chore: use SSA parser in all mem2reg tests
(noir-lang/noir#9372)
fix: trait where clause check fixes
(noir-lang/noir#9369)
fix: Correct doc comments for SSA passes
(noir-lang/noir#9371)
fix: prevent `SignedField::from(i128::MIN)` from crashing
(noir-lang/noir#9366)
fix: allow constants in the type-system to be negative
(noir-lang/noir#9360)
feat: show circuit output as a value of the program's return type
(noir-lang/noir#9364)
feat: add `FunctionDefinition::visibility`
(noir-lang/noir#9363)
chore(docs): Add example for `$crate` in docs
(noir-lang/noir#9361)
fix: Prevent accidental tuple sharing in comptime code
(noir-lang/noir#9313)
fix: perserve purities after SSA normalization
(noir-lang/noir#9355)
fix: modulo overflow in comptime
(noir-lang/noir#9348)
fix: handle short-syntax for trait constraints on trait generics
(noir-lang/noir#9167)
chore: enhance trait constraint comment
(noir-lang/noir#9358)
fix: replace implicitly added named generics with fresh type vars in
check_trait_impl_where_clause_matches_trait_where_clause
(noir-lang/noir#9352)
fix: push definition trait constraints after trait item constraint
(noir-lang/noir#9354)
chore(ci): Update status of noir_json_parser
(noir-lang/noir#9351)
fix(ssa): Keep reference count increments for array set values
(noir-lang/noir#9344)
chore: remove unused `compile_workspace`
(noir-lang/noir#9353)
chore: try printing byte arrays as strings in the SSA interpreter
(noir-lang/noir#9346)
feat(lsp): allow opening noir stdlib files
(noir-lang/noir#9339)
fix: do u128 operations with u128, not i128
(noir-lang/noir#9345)
chore(acir): ACIR parser error handling for blackbox inputs/outputs
(noir-lang/noir#9342)
fix: prevent invalid types in test/fuzz functions
(noir-lang/noir#9343)
chore(lsp): avoid redundant type checking
(noir-lang/noir#9337)
feat(acir): Parse ACIR memory and call opcodes
(noir-lang/noir#9331)
fix(ssa_gen): Add constraint on slice length before popping
(noir-lang/noir#9323)
chore: impl for u16 conversions
(noir-lang/noir#9314)
fix: substitute bindings in type before canonicalization
(noir-lang/noir#9328)
fix(ssa_interpreter): `push_back` and `pop_back` to slices with padding
(noir-lang/noir#9320)
fix: wildcard type should be allowed in lambda parameter types
(noir-lang/noir#9325)
chore: graceful handling of SIGPIPE
(noir-lang/noir#9075)
feat: return unsolvable opcode from `CircuitSimulator`
(noir-lang/noir#8943)
fix: allow nested fmtstr (noir-lang/noir#9309)
feat: Initial ACIR parser (arithmetic exprs and black box functions)
(noir-lang/noir#9316)
fix(mem2reg): Register aliases when the `IfElse` result in a reference
(noir-lang/noir#9305)
fix: Make Ssa-gen use existing reference when compiling `&mut
foo.bar.baz` (noir-lang/noir#9307)
fix: top-level item in dependency isn't always visible
(noir-lang/noir#9295)
fix(ssa-interpreter): Return error if slice length is 0 during popping
(noir-lang/noir#9308)
chore: Release Noir(1.0.0-beta.9)
(noir-lang/noir#9184)
chore(LSP): simplify code lens request handling
(noir-lang/noir#9279)
chore: add regression tests for #6383
(noir-lang/noir#9302)
fix: disallow `_` in signatures and struct members
(noir-lang/noir#9301)
fix: check associated types after validating where clause when looking
up trait impls, plus some unification fixes
(noir-lang/noir#9265)
chore: Add fmtstr to coercions list
(noir-lang/noir#9300)
chore: Add a helper function `fmtstr::as_quoted_str`
(noir-lang/noir#9293)
chore(docs): Copy Type Coercions docs into v1.0.0-beta.8 versioned docs
(noir-lang/noir#9298)
feat: only inject "out of bounds" checks in brillig
(noir-lang/noir#9200)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <tech@aztecprotocol.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants