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.

Sign up for GitHub

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

Jump to bottom

Salsify the crate graph #19337

Merged
merged 1 commit into from
Mar 12, 2025
Merged

Salsify the crate graph #19337

merged 1 commit into from
Mar 12, 2025
+3,629 −2,511

Conversation

ChayimFriedman2
Copy link
Contributor

I.e. make it not one giant input but multiple, for incrementality and decreased memory usage for Salsa 3 reasons.

This means that changing the metadata of a crate (e.g. adding a dependency) invalidates only it.

Draft for now because I still need to review the after-rebase changes.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 10, 2025
@ChayimFriedman2 ChayimFriedman2 force-pushed the salsify-crate-graph-final branch 2 times, most recently from c4f6156 to 57d1198 Compare March 11, 2025 23:46
@ChayimFriedman2
Copy link
Contributor Author

ChayimFriedman2 commented Mar 11, 2025 •
edited
Loading

Okay, this is now ready for review.

There are three big changes in this PR (I should've probably split them to commits, but now it is too late):

  1. Changing the crate graph to be incremental. The changes are mostly in base-db, and also spread in a lot of small changes all over the codebase where we access crate metadata.
  2. Splitting the "local" def map out of the def map, to allow for better incrementality for things like the extern prelude. This changes mostly in hir-def, in nameres.
  3. Loading proc macros and build scripts and only then setting the crate graph, to avoid invalidations. This change is in the rust-analyzer crate.

@ChayimFriedman2 ChayimFriedman2 marked this pull request as ready for review March 11, 2025 23:50
@ChayimFriedman2 ChayimFriedman2 force-pushed the salsify-crate-graph-final branch from 57d1198 to fdbad6e Compare March 12, 2025 13:50
davidbarsky
davidbarsky approved these changes Mar 12, 2025
View reviewed changes
Copy link
Contributor

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

mostly a few nits, but the changes look good to me. I mostly scanned all the usage site change, but nothing really stood out to me.

@ChayimFriedman2 ChayimFriedman2 force-pushed the salsify-crate-graph-final branch 4 times, most recently from abed0fb to 6b884f8 Compare March 12, 2025 18:47
@ChayimFriedman2
Salsify the crate graph
c94e9ef 
I.e. make it not one giant input but multiple, for incrementality and decreased memory usage for Salsa 3 reasons.
@ChayimFriedman2 ChayimFriedman2 force-pushed the salsify-crate-graph-final branch from 6b884f8 to c94e9ef Compare March 12, 2025 19:02
@ChayimFriedman2 ChayimFriedman2 added this pull request to the merge queue Mar 12, 2025
Merged via the queue into rust-lang:master with commit 3fc655b Mar 12, 2025
9 checks passed
@ChayimFriedman2 ChayimFriedman2 deleted the salsify-crate-graph-final branch March 12, 2025 19:35
@ShoyuVanilla ShoyuVanilla mentioned this pull request Mar 13, 2025
Merged
@ChayimFriedman2 ChayimFriedman2 mentioned this pull request Mar 13, 2025
Open
@ChayimFriedman2 ChayimFriedman2 mentioned this pull request Mar 27, 2025
Closed
@davidbarsky davidbarsky mentioned this pull request Mar 31, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbarsky davidbarsky davidbarsky approved these changes

Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ChayimFriedman2 @davidbarsky @rustbot