Improve efficiency of go to implementation for non workspace symbols

[tgodzik]

Currently, when looking for implementations of non workspace sources like RuntimeException, we load all information about the classpath.

To make if faster we could use https://github.com/classgraph/classgraph

Otherwise some hand crafted solution might be needed.

[joriscode]

Hi @tgodzik, I know very little of the code base of the project for the moment but I'm interested in this ticket. I'm currently reading metals doc, and https://scalameta.org/docs/semanticdb/specification.html#symbol. Is ImplementationProvider.implementations the correct entrypoint to this ticket?

[tgodzik]

@joriscode Awesome! This is quite an isolated issue, so you are welcome to take a look.

Most things that need to modified are in GlobalClassTable - it creates the context for situations where we need information about the dependencies inheritance. So that we for example know that RuntimeException inherits from Exception.

What we want is to replace the GlobalSymbolTable with just information about the class inheritance that we could get via classgraph.

The workflow I would recommend is running the ImplementationLspSuite.java-classes test which actually checks the functionality for non workspace symbols.

sbt unit/testOnly --  tests.ImplementationLspSuite.java-classes

We want to first confirm that we can get a much better performance from classgraph - but I am almost 100% sure that current implementation is way too slow.

Let me know if you need any more info!

[tgodzik]

@joriscode did you manage to take a look at this? Or is it a harder problem than we thought?

[joriscode]

@tgodzik Tbh, I had little time to look at it and I won't have more time for the coming week. However, I'm still interested in solving this issue. It goes without saying, that if this is blocking somehow the project or someone, I happily withdraw myself from the issue.

[tgodzik]

@joriscode It's not blocking, but I am worried that as a result of #1388 finding references might get slower. However, I doubt if anyone else will be able to take a look at it.

Also, I did some thinking and I suppose what we need to do here is to load information about class signatures from classgraph and translate them to semanticdb ClassSignature complete with filed/method signatures.

[pvid]

Hi @tgodzik! I've been looking into this a bit - I don't think that switching to classgraph would be a good approach if you'd want to collect ClassSignature objects. Reimplementing a translation from classgraph objects to semanticdb could be quite error prone and hard to maintain.

How about creating an eager and parallel alternative to the lazy GlobalSymbolTable? It could reuse scalameta implementation of classfiles parsing and take inspiration from classgraph.

BTW, do you have any specific goals in mind? Like a specific benchmark or memory usage goal that you'd like to achieve?

[tgodzik]

Hey @pvid, thanks for the interest!

Classgraph was a long shot anyway, we were mostly just curious if it would help out here. What might be useful is the new field that we added to semanticdb, which points to all parent symbols of a certain symbol (in the SymbolInformation class). It would greatly simplify the overall logic we use and would solve probably our issues with finding references to all overloaded symbols.

What we could do is to keep an additional cache of symbol parents and always search for all of those symbols when searching for references. Using that the go to implementation logic would be simplified plus also go to parent lenses. I haven't had a time to look at it but I think the steps should be:

1. Fix go to parent code lenses to use the new parent information (related issues Go to parent code lens use too much CPU #1645) - this would probably be mostly about removing a bunch of code and enabling the lenses
2. Add the new parent symbols cache during indexing in the ReferencesProvider and use all those symbols when searching for references (toString, hashcode etc. might be hardcoded)
3. At the same time we would need to make rename use references only for a single symbol (we currently also try to search the implementations, which would not be needed when 2. got implemented)
4. We could also greatly simplify go to implementation logic when searching for specific methods's implementation (we would just search all the class definition that has a required parent)

This is all a lot of work, but can be done over multiple steps and I think is much more doable than implementing the new global cache. It should also greatly improve the efficiency of multiple things.

I you need some more clarification about anything do let me know!

[pvid]

@tgodzik thanks for the write up! I can’t promise anything timewise, but I will certainly look into this if I have any free time

[tgodzik]

@tgodzik thanks for the write up! I can’t promise anything timewise, but I will certainly look into this if I have any free time

No worries! A lot have changed since I created the issue so I wanted to rethink what actually needs to be done and write it down here. There is a bunch of things that can be done separately here, so I think this might be more doable.