Skip to content

Comments

[ty] Make check_file a salsa query#19255

Merged
dhruvmanila merged 1 commit intomainfrom
micha/check-file-query
Jul 10, 2025
Merged

[ty] Make check_file a salsa query#19255
dhruvmanila merged 1 commit intomainfrom
micha/check-file-query

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jul 10, 2025 •
edited
Loading

Summary

We noticed that all files get reparsed when workspace diagnostics are enabled.

I realised that this is because check_file_impl access the parsed module but itself isn't a salsa query.
This pr makes check_file_impl a salsa query, so that we only access the parsed_module when the file actually changed. I decided to remove the salsa query from check_types because most functions it calls are salsa queries itself and having both check_types and check_file as salsa querise has the downside that we double cache the diagnostics.

Test Plan

Before

2025-07-10 12:54:16.620766000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c0c))}: File `/Users/micha/astral/test/yaml/yaml-stubs/__init__.pyi` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.621942000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c13))}: File `/Users/micha/astral/test/ignore2 2/nested-repository/main.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.622107000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c09))}: File `/Users/micha/astral/test/notebook.ipynb` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.622357000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c04))}: File `/Users/micha/astral/test/no-trailing.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.622634000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c02))}: File `/Users/micha/astral/test/simple.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.623056000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c07))}: File `/Users/micha/astral/test/open/more.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.623254000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c11))}: File `/Users/micha/astral/test/ignore-bug/backend/src/subdir/log/some_logging_lib.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.623450000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c0f))}: File `/Users/micha/astral/test/yaml/tomllib/__init__.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.624599000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c05))}: File `/Users/micha/astral/test/create.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.624784000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c00))}: File `/Users/micha/astral/test/lib.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.624911000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c0a))}: File `/Users/micha/astral/test/sub/test.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.625032000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c12))}: File `/Users/micha/astral/test/ignore2/nested-repository/main.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.625101000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c08))}: File `/Users/micha/astral/test/open/test.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.625227000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c03))}: File `/Users/micha/astral/test/pseudocode_with_bom.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.625353000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c0b))}: File `/Users/micha/astral/test/yaml/yaml-stubs/loader.pyi` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.625543000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c01))}: File `/Users/micha/astral/test/test_trailing.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.625616000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c0d))}: File `/Users/micha/astral/test/yaml/tomllib/_re.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.625667000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c06))}: File `/Users/micha/astral/test/yaml/main.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.625779000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c10))}: File `/Users/micha/astral/test/yaml/tomllib/_types.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.627526000  WARN request{id=19 method="workspace/diagnostic"}:Project::check:check_file{file=file(Id(c0e))}: File `/Users/micha/astral/test/yaml/tomllib/_parser.py` was reparsed after being collected in the current Salsa revision
2025-07-10 12:54:16.627959000 DEBUG request{id=19 method="workspace/diagnostic"}:Project::check: Checking all files took 0.007s

Now, no more logs regarding reparsing

@MichaReiser MichaReiser added internal An internal refactor or improvement ty Multi-file analysis & type inference labels Jul 10, 2025
@MichaReiser MichaReiser force-pushed the micha/check-file-query branch from 96c9711 to 4b96afb Compare July 10, 2025 10:49
@github-actions
Copy link
Contributor

github-actions bot commented Jul 10, 2025 •
edited
Loading

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@MichaReiser MichaReiser force-pushed the micha/check-file-query branch from 4b96afb to 0a5519b Compare July 10, 2025 10:56
@MichaReiser MichaReiser mentioned this pull request Jul 10, 2025
Closed
dhruvmanila
dhruvmanila approved these changes Jul 10, 2025
View reviewed changes
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Thanks!

@dhruvmanila dhruvmanila merged commit 87f6f08 into main Jul 10, 2025
37 checks passed
@dhruvmanila dhruvmanila deleted the micha/check-file-query branch July 10, 2025 13:16
UnboundVariable pushed a commit to UnboundVariable/ruff that referenced this pull request Jul 11, 2025
Merge branch 'main' of https://github.com/astral-sh/ruff into signatu…
94c9bfe 
…re_help

* 'main' of https://github.com/astral-sh/ruff:
  Add simple integration tests for all output formats (astral-sh#19265)
  [`flake8-return`] Fix false-positive for variables used inside nested functions in `RET504`  (astral-sh#18433)
  [ty] Add a `--quiet` mode (astral-sh#19233)
  Treat form feed as valid whitespace before a line continuation (astral-sh#19220)
  [ty] Make `check_file` a salsa query (astral-sh#19255)
  [ty] Consolidate submodule resolving code between `types.rs` and `ide_support.rs` (astral-sh#19256)
  [ty] Remove countme from salsa-structs (astral-sh#19257)
  [ty] Improve and document equivalence for module-literal types (astral-sh#19243)
  [ty] Optimize protocol subtyping by removing expensive and unnecessary equivalence check from the top of `Type::has_relation_to()` (astral-sh#19230)
  [ty] Ecosystem analyzer: parallelize, fix race condition (astral-sh#19252)
  [ty] Add completion kind to playground (astral-sh#19251)
  [ty] Deploy ecosystem diff to Cloudflare pages (astral-sh#19234)
  [ty] Add semantic token provider to playground (astral-sh#19232)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dhruvmanila dhruvmanila dhruvmanila approved these changes

@carljm carljm Awaiting requested review from carljm carljm is a code owner

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood AlexWaygood is a code owner

@sharkdp sharkdp Awaiting requested review from sharkdp sharkdp is a code owner

@dcreager dcreager Awaiting requested review from dcreager dcreager is a code owner

Assignees

No one assigned

Labels

internal An internal refactor or improvement ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MichaReiser @dhruvmanila