Skip to content

Give non-existent files a durability of at least Medium#14034

Merged
MichaReiser merged 1 commit intomainfrom
micha/file-status-durability
Nov 1, 2024
Merged

Give non-existent files a durability of at least Medium#14034
MichaReiser merged 1 commit intomainfrom
micha/file-status-durability

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Nov 1, 2024

Summary

Partially addresses astral-sh/ty#242

The solution isn't ideal, but our problem is that the module resolves first queries if certain first-party files exist and only then resolves the standard library modules. The issue with that is that any query that depends on a resolved standard library module now depends on non-existing LOW durability files, negating the benefit of giving standard library files high durability.

This PR changes the durability of the File::status to at least medium. This is still not perfect because it means that all standard library queries that depend on module resolution still have a MEDIUM durability instead of HIGH, but it's better than what we have today, and it should allow Salsa to take the "fast-incremental path" more often.

This possibly helps with #14024

Test Plan

See benchmarks

@MichaReiser MichaReiser added the ty Multi-file analysis & type inference label Nov 1, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 1, 2024

CodSpeed Performance Report

Merging #14034 will improve performances by 7.43%

Comparing micha/file-status-durability (c485dce) with main (48fa839)

Summary

⚡ 1 improvements
✅ 31 untouched benchmarks

Benchmarks breakdown

Benchmark main micha/file-status-durability Change
red_knot_check_file[incremental] 4.6 ms 4.3 ms +7.43%

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

So just to make sure I understand the effect here: module resolutions that only use typeshed files (but first have to verify that some first-party paths don't exist) will now be MEDIUM instead of LOW durability, meaning we won't have to re-resolve those paths on any change to a first-party file. Adding a previously-not-existent first-party file will count as a MEDIUM durability input change, and cause those queries to have to re-execute. Is that an accurate summary?

Looks good!

@MichaReiser
Copy link
Member Author

So just to make sure I understand the effect here: module resolutions that only use typeshed files (but first have to verify that some first-party paths don't exist) will now be MEDIUM instead of LOW durability, meaning we won't have to re-resolve those paths on any change to a first-party file. Adding a previously-not-existent first-party file will count as a MEDIUM durability input change, and cause those queries to have to re-execute. Is that an accurate summary?

Looks good!

Yes, that's a perfect summary.

@MichaReiser MichaReiser merged commit 8574751 into main Nov 1, 2024
@MichaReiser MichaReiser deleted the micha/file-status-durability branch November 1, 2024 15:44
TomerBin pushed a commit to TomerBin/ruff that referenced this pull request Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants