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

Add option to disable incremental compilation #213

Merged
merged 1 commit into from Feb 8, 2017

Conversation

romanowski
Copy link
Contributor

@romanowski romanowski commented Jan 27, 2017

Now it is possible to disable incremental compilation metadata creation in IncOptions and directly in AnalysisCallback.

With this flag on e.g. Intellij scala plugin can migrate to new zinc (since it also uses zinc to in it's own incremental compiler but with zinc incremental compilation off)

@@ -218,8 +218,10 @@ private final class CachedCompiler0(args: Array[String], output: Output, initial
override lazy val phaseDescriptors =
{
phasesSet += sbtAnalyzer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You say about AnalysisCallback#enabled "If returns false, no sbt phases will be introduced in compiler". Why is sbtAnalyzer still added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'''sbtAnalyzer''' only reports generated classes and without it end users will needs to find them manually (e.g. scanning output).

Main (and only TBH) reason to disable incremental compilation is not to pay overhead (~5% of compilation time after my recent changes) and '''sbtAnalyzer''' almost not affects compilation times.

I will change wording of scala doc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I don't know zinc very well. I wasn't criticising - just learning :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was good point (the problem was that I created this doc before I realized that I need xsbt-analyzer)

Now it is possible to disable incremental compilation metadata creation in
IncOptions and directly in AnalysisCallback.
@romanowski romanowski force-pushed the turn-off-incremental-compilation branch from 75baad1 to f898c2b Compare January 27, 2017 11:51
@jvican
Copy link
Member

jvican commented Feb 2, 2017

@romanowski A small question: why is Intellij using Zinc and disabling incremental compilation? Concrete use case?

@romanowski
Copy link
Contributor Author

Yes. Intellij is using zinc as a scalac wrapper in its own incremental compiler (and then there is no point for using making compilation incremental).

IIRC Intellij uses custom build of sbt (based on 0.13.5) and has this change included.
https://github.com/JetBrains/intellij-scala/blob/idea163.x/project/dependencies.scala#L44-L48

@jastice @niktrop @mutcianm can you shade some light here?

@niktrop
Copy link
Contributor

niktrop commented Feb 3, 2017

Currently we use a patched version of Zinc from sbt 0.13.9. It allows us to use different versions of scala compiler in the same way and provides an API for getting feedback from it.

https://github.com/niktrop/sbt/commits/0.13.9.patched

There are two modes of incremental compilation supported in IDEA. One just uses Zinc, second is adapted from IDEA incremental compiler for java, based on bytecode analysis. So the main reason for use a patched version is to get all information required for IDEA incremental compiler. Other reason is indeed to skip compiler phases introduced to collect data for Zinc when we do not need it.

@jvican
Copy link
Member

jvican commented Feb 3, 2017

Interesting @niktrop, thanks for elaborating on it. So another question: is the IDEA incremental compiler using bytecode analysis in Scala class files or only Java class files?

@niktrop
Copy link
Contributor

niktrop commented Feb 4, 2017

It works for any class files. We just have to provide a correspondence between sources and compiled classes. I think we do some additional checks for scala package objects, but thats all.

@jvican
Copy link
Member

jvican commented Feb 4, 2017

@niktrop AFAIK, class file analysis won't give you information about a lot of Scala dependencies because some of them are not even present in the generated class files, especially those depending on types. Is this a known limitation in your incremental compiler and you intentionally not cover those cases?

@niktrop
Copy link
Contributor

niktrop commented Feb 4, 2017

We are aware of it. There is a tradeoff between correctness and completeness of dependency analysis and recompilation time. So IDEA incremental compiler for Scala is optimistic one and it works rather well. It was simple to implement using IDEA platform, but we didn't mean to create a full fledged alternative to Zinc.

@jvican
Copy link
Member

jvican commented Feb 4, 2017

Yeah, I thought so @niktrop. that makes sense -- Just wanted to confirm this is the case. Thanks for elaborating on it.

@eed3si9n
Copy link
Member

eed3si9n commented Feb 8, 2017

The change looks good to me. Can I merge it?

@eed3si9n eed3si9n merged commit 7cad702 into sbt:1.0 Feb 8, 2017
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 this pull request may close these issues.

None yet

5 participants