Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Don't run compilation on InteractiveDriver.run every time for the "same input" #17715

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

Closed
tanishiking opened this issue Jul 7, 2022 · 7 comments

Comments

@tanishiking
Copy link
Member

Originally reported in scalameta/metals#4060

TL;DR

  • I'm looking for a way to cache the result of InteractiveDriver.run
  • I believe we can invalidate the cache by checking
    • input file isn't updated from the last compilation
    • there's no change in the classpath from the last compilation
  • If yes, where should I handle it? In InteractiveDriver or its consumers (like metals side)?
  • If no, what we need to check for cache invalidation?

Background

While I CPU profile Metals using async-profiler, I realized that Metals (in Scala3 project) invokes InteractiveDriver.run quite frequently, especially from HoverProvider.hover (that invokes when users hover on symbol in the editors).

This is because we invoke InteractiveDriver.run every time when Metals uses information from presentation compilers.
For example, HoverProvider.hover runs InteractiveDriver.run for every invocations anyway.
https://github.com/scalameta/metals/blob/eab3df81d3a55445f558c959ee5d2fa24308be82/mtags/src/main/scala-3/scala/meta/internal/pc/HoverProvider.scala#L39

It might be a big CPU saver if we don't run InteractiveDriver.run if the input is the same (that is likely to happen especially when we're code reading and doing hover or navigations).

A little bit of experiments

I tried to experiment, maybe we can cache invalidate by checking the combination of file content and runId scalameta/metals#4100

However, I realized that this work only if we edit files only in Metals.

// initial
// src/main/scala/A.scala
trait A {
  def foo: Int
}
// src/main/scala/B.scala
object B:
  val a: A = _
  val v = a.foo // call hover on `v` - should show `Int`
  
// then do changes in A.scala outside of Metals (which means there's no compilation in InteractiveDriver, but it should be compiled by the build server like bloop or sbt and classpath will be updated)
// src/main/scala/A.scala
trait A {
  def foo: String
}
// src/main/scala/B.scala
object B:
  val a: A = _
  val v@@ = a.foo // it should show `String` 
                  // but if there's a cache by file content of `B.scala` and `runId`
                  // it will show `Int` on hover.

We need to know if there are changes on the classpath. 🤔

Questions

  • I believe we can cache invalidate by the file content and checking there was a change on classpath that InteractiveDriver aware of, no?
  • If yes, where should we handle it, InteractiveDriver side or Metals side? (Or do you think we shouldn't cache it?)
  • If no, what we should check in addition to the classpath and content?

Any opinions are welcome!

BTW, I heard that someone is already taking a look at this topic, maybe @jchyb ? (sorry if I mistaken)

@jchyb
Copy link
Contributor

jchyb commented Jul 7, 2022

Wasn't me, although I did do something similar some time ago for Scala Native sbt plugin (which, in hindsight, maybe should have been included in the Scala Native itself), where we ended up caching classpath file content hashes and the configuration. Not sure if I can help with this though (no knowledge about the InteractiveDriver)

@tanishiking
Copy link
Member Author

Sorry, it was @rochala 😆

@rochala
Copy link
Contributor

rochala commented Jul 8, 2022

Yes, indeed I was going (and still I plan on doing it) to look more into this topic, but I didn't have time recently. I'll do this probably next week.

@vzmerr
Copy link

vzmerr commented Aug 1, 2022

I also have a problem with this issue. The laptop gets so hot because of it that it burns my fingers.

@tanishiking
Copy link
Member Author

We discussed this feature with @rochala, and he built some PoC implementation.
We thought it could sufficiently invalidate the cache by checking the classpaths' timestamp and skip compilation if nothing has changed since the last compilation of the given files. However, it turns out it sometimes classpaths' timestamp invalidation takes more time than InteractiveDriver.run itself. We might need another option to cache the compilation result.

Another idea is implementing the InteractiveDriver cache on the Metals side:

  • Metals has a FileWatcher feature that detect the file adding/modification/deletion in workspace and re-compile the related projects.
  • We can clean up the cache when we detect the file change
    • More precisely, we can clean up the caches of files under the affected projects

@tanishiking
Copy link
Member Author

Hey @rochala I figured out how Scala2 + Metals caches the compilation result, I summarized https://contributors.scala-lang.org/t/how-does-scala2-compiler-cache-invalidate-the-typechecking-result-for-the-given-tree/5858/4

While in Scala2, we skip typechecking if the given tree is already typed, it looks like Scala3 doesn't do such a thing (and it might be a bit complicated to do that). Therefore, I'm going to implement the cache feature in Metals basically the way I described in scalameta/metals#4060 + cache purging on change.

Thank you very much for taking a look at this issue from the perspective of the compiler side / discussing it with me!

@tanishiking
Copy link
Member Author

End up doing like this scalameta/metals#4225 :) FYI @rochala

@ckipp01 ckipp01 transferred this issue from lampepfl/dotty-feature-requests May 31, 2023
@scala scala locked and limited conversation to collaborators May 31, 2023
@ckipp01 ckipp01 converted this issue into discussion #17716 May 31, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants