internal: Make use of salsa's new NEVER_CHANGE durability#22638
internal: Make use of salsa's new NEVER_CHANGE durability#22638Veykril wants to merge 3 commits into
Conversation
rustbot
added
the
S-waiting-on-review
| ) { | ||
| match self.files.entry(file_id) { | ||
| Entry::Occupied(mut occupied) => { | ||
| Entry::Occupied(mut occupied) if durability != Durability::NEVER_CHANGE => { |
There was a problem hiding this comment.
Ignoring the write seems a bit suspicious. Isn't it desired to panic here and, instead, either fix the durability for the file if it is NEVER_CHANGE and can change, or change the call site not to call set_file_text_with_durability for such a fiel?
There was a problem hiding this comment.
You are not wrong, but our file setup is kind of convoluted so I don't even know if we can reliably do that.
There was a problem hiding this comment.
In fact yea then we'd need to do the filtering somewhere else as we do track changes to these files (which arguably we shouldn't register file watchers for I think)
|
If this has the same disadvantage as my old branch, namely that it cannot reload changed features in dependencies correctly, then I oppose to that. We're not at the same place we were at the time wrt. memory usage, and today I think this is not worth the code complexity and workflow disruption. |
|
In ty, we only use it for fields that are truly immutable. That is, we vendor some files, so they can never change. Each file has a |
|
We don't have such things, but we do have std which I suppose we can assume never changes. It won't be a big win, but given this is already in Salsa I guess it could be worth it. |
|
But we do? Any file in the local crate registry as well as the standard library we can assume to be never changing. That's what this PR is doing. Aside from that I agree, there isn't really anything else (aside from some source roots in theory maybe?). |
|
No, like I said back then, changing Cargo features (or cfgs) can change the analysis on those files, even if their content doesn't change. |
|
I see now that you don't mark the |
|
Yea, trusting the analysis stats run here its ~4mb on self. Not great but might as well try. I do want to check out how much using on a crate would save (just out of curiosity), its a shame we can't actually do that. |
|
Do note that this is not completely enough, you also need to make the |
rustbot
removed
the
S-waiting-on-review
4 participants
No description provided.