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

Go to definition does not work or is slow #7209

Open
odersky opened this issue Feb 13, 2025 · 11 comments · May be fixed by #7210
Open

Go to definition does not work or is slow #7209

odersky opened this issue Feb 13, 2025 · 11 comments · May be fixed by #7210

Comments

@odersky
Copy link
Collaborator

odersky commented Feb 13, 2025

Since upgrading to Metals 1.5.1, the cmd-hover functionality does not work anymore or is VERY slow. Something like 5-10 seconds to go from a use to a definition 5 lines prior. About half of the times I never get an underline, the other half it takes 5-10 seconds. If I choose "go to definition" in the menu it seems to work in most or all cases (unlike cmd-hover), but it also takes 5-10 seconds. Needless to say, that makes Metals pretty annoying for development.

Expected behaviour:

Operating system:
Mac OS X

Java version:
17.0.13

Editor/extension:
Visual Studio Code v1.97.0

Metals version:
1.5.1

Extra context or search terms:

Workspace information:

  • Scala versions: 3.6.4-RC1; 2.12.20; 2.13.16
  • Build tools: 0. sbt
  • Build servers:
    0. sbt v1.10.5
  • All build tools in workspace: Bloop; sbt
@tgodzik
Copy link
Contributor

tgodzik commented Feb 13, 2025

Thanks for reporting, this might have something to do with the changed priority. We would use semanticdb first previously, but we switched to using the compiler itself to make sure we get a more correct result. @kasiaMarek could that be it?

Maybe we should do a race instead? 🤔

@tgodzik
Copy link
Contributor

tgodzik commented Feb 13, 2025

Does this happen in any particular file?

@odersky
Copy link
Collaborator Author

odersky commented Feb 13, 2025

It seems to happen in all files of the current Scala 3 compiler. Let me try to checkout main. to see whether it's in only my branch.

@tgodzik
Copy link
Contributor

tgodzik commented Feb 13, 2025

Might be already fixed but for main branch and not on the RC scala/scala3#22491 and in particular #7105

@tgodzik
Copy link
Contributor

tgodzik commented Feb 13, 2025

You could revert to 1.5.0 for now until we are on 3.7.0 RCs

@tgodzik
Copy link
Contributor

tgodzik commented Feb 13, 2025

Alternatively, we can make the behaviour different on versions prior to 3.7.0.

@odersky
Copy link
Collaborator Author

odersky commented Feb 13, 2025

It looks like it is the same for main branch for me. The fact that it takes so long if the presentation compiler is used rather than semanticDb seems to indicate some wrong usage of the presentation compiler. I mean, the compiler can compile the source file about 10 times from scratch until a find definition finds the symbol. So something's definitely amiss here.

@kasiaMarek
Copy link
Contributor

kasiaMarek commented Feb 13, 2025

We would use semanticdb first previously, but we switched to using the compiler itself to make sure we get a more correct result. @kasiaMarek could that be it?

Yeah, sounds like it...

Might be already fixed but for main branch and not on the RC scala/scala3#22491 and in particular #7105

That should make it much faster but it's only for symbols defined within the same file. If that issue affects also other go to definition requests we need to rethink the strategy here.

The fact that it takes so long if the presentation compiler is used rather than semanticDb seems to indicate some wrong usage of the presentation compiler.

That is the #7105 fix. We searched way too much for the definition.

@odersky
Copy link
Collaborator Author

odersky commented Feb 13, 2025

I just verified it's the same for 1.5.0. It's really slow for files that are currently open. It's a bit faster for definitions that get accessed through Tasty, it seems. And it seems to affect all files in the Scala 3 project (which is about 100K lines total).

That is the #7105 fix. We searched way too much for the definition.

Good that this was fixed! In general I am all for using the PC more.

@tgodzik
Copy link
Contributor

tgodzik commented Feb 13, 2025

Ach right, we did two release quickly, so you would need to go back to 1.4.2

tgodzik added a commit to tgodzik/metals that referenced this issue Feb 13, 2025
@odersky
Copy link
Collaborator Author

odersky commented Feb 13, 2025

Yes, 1.4.2 works. I am staying on that one for now.

tgodzik added a commit to tgodzik/metals that referenced this issue Feb 13, 2025
tgodzik added a commit to tgodzik/metals that referenced this issue Feb 13, 2025
tgodzik added a commit to tgodzik/metals that referenced this issue Feb 14, 2025
tgodzik added a commit to tgodzik/metals that referenced this issue Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants