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.

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

Many unnecessary allocations in path resolution #15924

Closed
Noratrieb opened this issue Nov 17, 2023 · 1 comment · Fixed by #15925
Closed

Many unnecessary allocations in path resolution #15924

Noratrieb opened this issue Nov 17, 2023 · 1 comment · Fixed by #15925
Labels
C-bug Category: bug

Comments

@Noratrieb
Copy link
Member

Noratrieb commented Nov 17, 2023

let _cx = stdx::panic_context::enter(format!(
"DefMap {:?} crate_name={:?} block={:?} path={}",
self.krate,
graph[self.krate].display_name,
self.block,
path.display(db.upcast())
));

This does a bunch of string formatting on the happy path. According to my dhat analysis (opening rust-lang/rust with RA running in dhat and then waiting for a few minutes (it is very slow)), this accounts for 13% of the allocated blocks inside rust-analyzer. I don't think it uses much memory, but it's probably quite slow. I don't think my analysis is necessarily representative of normal usage, as it only really captured startup, but it still seems pretty relevant.

image

rust-analyzer version: current master bc97821

rustc version: 1.76.0-nightly, doesn't matter

relevant settings: the recommended settings for rust-lang/rust, provided there in src/etc/rust_analyzer_settings.json

@Noratrieb Noratrieb added the C-bug Category: bug label Nov 17, 2023
@Veykril
Copy link
Member

Veykril commented Nov 17, 2023

We can just remove those, they were added to aid in debugging #10084 which we now consider an undeterministic bug of salsa, so they don't really help anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants