incremental refactorings
fommil opened this issue · 13 comments
Ensime is experiencing this deficiency which may be due to the way that we are using this library: https://github.com/ensime/ensime-src/issues/362
Can you please advise how we could avoid re parsing/type checking everything?
I fear there's no easy solution: refactorings need the fully type-checked trees. In Eclipse, I mitigate this by trying to figure out what a refactoring is going to touch and then only type-check those files (and the Scala IDE itself already keeps the opened files up to date).
@misto in ensime this is in the context of editing a single file. The type checker is checking the entire tree instead of just the modified file. Is there any way to avoid rechecking the dependencies?
Presumably the presentation compiler is already keeping a list of which files depend on which files, so we should know how to invalidate caches for simple changes. The SBT incremental compiler is doing the same in a separate JVM, which feels wasteful (but a secondary concern for me at this stage).
Ensime has a lisp function to invalidate all caches, I'm happy using that to work around non-trivial dependency problems.
While I looked at Ensime's code to get an idea where it does the typechecking before the refactoring, I found something interesting:
https://github.com/ensime/ensime-src/blob/master/src/main/scala/org/ensime/server/Refactoring.scala#L194
If I read this correctly, this creates an index of all the active units? That's not necessary if the refactorings only perform local work, and might cause a lot of overhead, maybe even trigger all the type checking.
@misto interesting. That could certainly be optimised. In my case the problem is that ensime is rechecking the types for the entire tree (ensime-typecheck-current-file(1)) and not just the local file (2) (with a ReloadFilesReq) and while the re-indexing is going on, we just get a non-informative error back.
To fix this, I'm hoping to:
- display a more sensible error message (e.g. "waiting for type checker to complete") (should be trivial, but repetitive)
- change the save-file hook to only type-check the current file and not its entire dependency tree.
But looking at the code at [2], I can't see how to call askReloadFiles and tell it not to go the whole way down the dependency tree. Am I missing something or is this the crux of the problem?
[1] https://github.com/ensime/ensime-src/blob/master/src/main/elisp/ensime.el#L2380
[2] https://github.com/ensime/ensime-src/blob/master/src/main/scala/org/ensime/server/Analyzer.scala#L160
One more thing. My project has a very high module dependency structure, so although the file I am editing may depend on hundreds of other files beneath, they are all mostly in other modules. Does that mean the presentation compiler has to go off and re-check everything in the entire classpath, not just the source level? I have a separate compiler process which ensures that the .class files are available.
@misto btw, please pass on the word about https://groups.google.com/d/msg/ensime/KkQo2vOQd8M/BAf8PM8m6BsJ
So it looks like you're using the compiler's askReload method: https://github.com/scala/scala/blob/2.11.x/src/interactive/scala/tools/nsc/interactive/CompilerControl.scala#L98 and this recompiles everything. What you could try instead is to use askLoadedTyped: https://github.com/scala/scala/blob/2.11.x/src/interactive/scala/tools/nsc/interactive/CompilerControl.scala#L188
The Eclipse Scala IDE only keeps the opened files in the presentation compiler, if a file is closed it is removed and not re-checked. If ENSIME keeps all the files in the project in the presentation compiler, that might explain some of the performance issues.
For class files, the compiler doesn't re-typecheck them, it just loads the class files and uses that information.
Is this available in the 2.9 branch?
Yes, that has been around for quite some time: https://github.com/scala/scala/blob/2.9.x/src/compiler/scala/tools/nsc/interactive/CompilerControl.scala#L187
@misto I updated the ensime code to use askLoadedTyped when reloading a single file, but it doesn't seem to do anything. e.g. the presentation compiler has a bug that doesn't allow this code:
log.info("blah blah " + url)
because it wants url to be a String (not a URL).
If I edit the text to read
log.info("blah blah " + url.toString)
nonExistentValue.muahahaha
I still get the pres-compiler artefact and the new nonsense text is ignored... it's like it doesn't know that the file changed on disk at all.
Hmm.. unfortunately I'm only familiar with the parts of the presentation compiler that I use for the refactorings, and there I don't work with actual files at all.
does that mean that askLoadedType doesn't check (or fire off) syntax errors?
btw, I'm closing this because it looks like its a presentation compiler thing.
askLoadedTyped fully type-checks the file and reports problems, so I'm a bit surprised you're not seeing the errors. I'm actually surprised that it works for you with askReload, because this only parses the file, and doesn't type check.