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

Finding all references to functions called new can take a long time #7404

Closed
umanwizard opened this issue Jan 22, 2021 · 6 comments · Fixed by #17927
Closed

Finding all references to functions called new can take a long time #7404

umanwizard opened this issue Jan 22, 2021 · 6 comments · Fixed by #17927
Labels
A-perf performance issues A-ty type system / type inference / traits / method resolution C-support Category: support questions E-hard S-actionable Someone could pick this issue up and work on it right now

Comments

@umanwizard
Copy link
Contributor

umanwizard commented Jan 22, 2021

Sorry for the Materialize dependency, but I think a large-ish codebase is probably necessary to repro this.

To repro:

  1. Check out https://github.com/MaterializeInc/materialize
  2. Navigate over fn new in impl Locald (as of this writing, it's in src/expr/src/id.rs:45
  3. Find all references
  4. Observe it taking about 60s.

Maybe since finding references involves filtering by a string first, the very common name is what makes things slow? Just a guess.

@jonas-schievink jonas-schievink added A-perf performance issues S-actionable Someone could pick this issue up and work on it right now labels Jan 22, 2021
@lnicola
Copy link
Member

lnicola commented Jan 24, 2021

Confirmed:

   80 textDocument/references             36873ms

Seems to burn CPU in chalk for some reason. The second "find all references" (on GlobalId::is_system) takes 63ms, though. and a third one (on ResolvedRecordAccess::new) in 1ms.

@lnicola lnicola added the A-ty type system / type inference / traits / method resolution label Jan 24, 2021
@matklad matklad added the E-hard label Jan 25, 2021
@matklad
Copy link
Member

matklad commented Jan 25, 2021

Seems to burn CPU in chalk for some reason.

It basically type-checks the world (every function that has a new call) :D. To fix this, we need to double-check that we are using the correct scope here and don't, eg look in the stdlib. Then, we either need to come up with some sound optimization here (like looking for the struct at first), or to do something extra-clever and run the search on resolved code (which we might need for macros)... Let me draft an issue...

@jplatte
Copy link
Contributor

jplatte commented Aug 25, 2022

Let me draft an issue...

Has this happened? I just came across how long it takes to find all references to a std item. Surely finding references in crates.io crates can be helpful and should probably have an option, but when do you actually want to find references in the standard library (except for the case of working on it)? 😅

@Veykril
Copy link
Member

Veykril commented Oct 4, 2022

It has, I believe it's #7427

@Veykril Veykril added the C-support Category: support questions label Feb 9, 2023
@alexkirsz
Copy link
Contributor

We're frequently running into this in our codebase, which is about 230 crates large. Any method with a common-enough name like new or map will take in the dozens of seconds, sometimes minutes, to complete.

@Veykril
Copy link
Member

Veykril commented Aug 23, 2024

This should have been hopefully improved by #17927

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-perf performance issues A-ty type system / type inference / traits / method resolution C-support Category: support questions E-hard S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
7 participants