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

unexpected transitive recompilation #288

Closed
retronym opened this issue Dec 5, 2011 · 11 comments
Labels
Bug

Comments

@retronym
Copy link
Member

@retronym retronym commented Dec 5, 2011

I've been unable to whittle this down to a small test case, but I can provide steps to reproduce.

> scalaz-seven ~/code/scalaz7-experimental cd ~/tmp
  ~/tmp git clone git@github.com:scalaz/scalaz.git
Cloning into scalaz...
remote: Counting objects: 31089, done.
remote: Compressing objects: 100% (6215/6215), done.
remote: Total 31089 (delta 17138), reused 30859 (delta 16954)
Receiving objects: 100% (31089/31089), 63.14 MiB | 460 KiB/s, done.
Resolving deltas: 100% (17138/17138), done.
  ~/tmp cd scalaz
  ~/tmp/scalaz git checkout 348794a811
Note: checking out '348794a811'.

HEAD is now at 348794a... Fix infinite loop.
(remotes/origin/scalaz-seven) ~/tmp/scalaz sbt compile
Detected sbt version 0.11.0
[info] Loading global plugins from /Users/jason/.sbt/plugins
[info] Loading project definition from /Users/jason/tmp/scalaz/project
[info] Updating {file:/Users/jason/tmp/scalaz/project/}default-ed0835...
[info] Done updating.
[info] Compiling 4 Scala sources to /Users/jason/tmp/scalaz/project/target/scala-2.9.1/sbt-0.11.0/classes...
[info] Set current project to scalaz (in build file:/Users/jason/tmp/scalaz/)
[info] Done updating.
[info] Compiling 6 Scala sources to /Users/jason/tmp/scalaz/concurrent/target/scala-2.9.1/classes...
[info] Compiling 31 Scala sources to /Users/jason/tmp/scalaz/effect/target/scala-2.9.1/classes...
[info] Compiling 3 Scala sources to /Users/jason/tmp/scalaz/scalacheck-binding/target/scala-2.9.1/classes...
[info] Compiling 7 Scala sources to /Users/jason/tmp/scalaz/iteratee/target/scala-2.9.1/classes...
[info] Compiling 14 Scala sources to /Users/jason/tmp/scalaz/example/target/scala-2.9.1/classes...
[success] Total time: 134 s, completed Dec 5, 2011 12:47:38 AM

(remotes/origin/scalaz-seven) ~/tmp/scalaz echo " " >> core/src/main/scala/scalaz/std/Stream.scala 

(remotes/origin/scalaz-seven) ~/tmp/scalaz sbt compile
Detected sbt version 0.11.0
[info] Loading global plugins from /Users/jason/.sbt/plugins
[info] Loading project definition from /Users/jason/tmp/scalaz/project
[info] Set current project to scalaz (in build file:/Users/jason/tmp/scalaz/)
[info] Compiling 1 Scala source to /Users/jason/tmp/scalaz/core/target/scala-2.9.1/classes...
[info] Compiling 152 Scala sources to /Users/jason/tmp/scalaz/core/target/scala-2.9.1/classes...

I expected only one file to be compiled in this case.

Replacing:

https://github.com/scalaz/scalaz/blob/348794a81186c847846f0e6d47930b5211bfa1c5/core/src/main/scala/scalaz/std/Stream.scala#L7

with

implicit val streamInstance: Traverse[Stream] with MonadPlus[Stream] with Each[Stream] with Index[Stream] with Length[Stream] =
    new Traverse[Stream] with MonadPlus[Stream] with Each[Stream] with Index[Stream] with Length[Stream] {

avoids the transitive recompiles.

I can reproduce with SBT 0.11.1, as well

(remotes/origin/scalaz-seven) ~/tmp/scalaz echo " " >> core/src/main/scala/scalaz/std/Stream.scala 

(remotes/origin/scalaz-seven) ~/tmp/scalaz sbt compileDetected sbt version 0.11.1
[info] Loading global plugins from /Users/jason/.sbt/plugins
[info] Loading project definition from /Users/jason/tmp/scalaz/project
[info] Set current project to scalaz (in build file:/Users/jason/tmp/scalaz/)
[info] Compiling 1 Scala source to /Users/jason/tmp/scalaz/core/target/scala-2.9.1/classes...
[info] Compiling 152 Scala sources to /Users/jason/tmp/scalaz/core/target/scala-2.9.1/classes...

And with 0.11.2:

(remotes/origin/scalaz-seven) ~/tmp/scalaz echo " " >> core/src/main/scala/scaz/std/Stream.scala 

(remotes/origin/scalaz-seven) ~/tmp/scalaz sbt -no-share compileDetected sbt version 0.11.2
[info] Loading project definition from /Users/jason/tmp/scalaz/project
[info] Set current project to scalaz (in build file:/Users/jason/tmp/scalaz/)
[info] Compiling 1 Scala source to /Users/jason/tmp/scalaz/core/target/scala-2.9.1/classes...
[info] Compiling 152 Scala sources to /Users/jason/tmp/scalaz/core/target/scala-2.9.1/classes...
@harrah

This comment has been minimized.

Copy link
Member

@harrah harrah commented May 1, 2012

It is possible this was fixed in a recent commit. The class involved there was Typers.scala from the compiler. We couldn't minimize that case either, so unfortunately there is no regression test for this, but at least it was fixed in that case. Please reopen if it is still a problem.

@harrah harrah closed this May 1, 2012
@asouza

This comment has been minimized.

Copy link

@asouza asouza commented May 12, 2012

Hi, maybe this is another issue. I have this code:

class Person(name:String,visitedCities:List[City] = List[City]())

class City(val name:String)

object Program extends App{
    println(new Person("alberto"))
}

When i change the City class adding one public variable, the sbt invalidates both Person and Program. Why does it recompile the Program class? I did not change the Person class... Is there some reason?

@retronym

This comment has been minimized.

Copy link
Member Author

@retronym retronym commented May 12, 2012

Are they in the same file? Invalidation granularity is based on files, not classes/objects/traits.

Background:

https://groups.google.com/d/msg/simple-build-tool/Lq2BafF1MBc/fCrNHUMnE4IJ

@asouza

This comment has been minimized.

Copy link

@asouza asouza commented May 13, 2012

No, they are in separated files... But, if when I add a private attribute, sbt just compiles the class that was changed. Anyway, I was thinking if it is a good idea that the sbt just compiles the directly dependencies instead of traverse the graph

@Blaisorblade

This comment has been minimized.

Copy link
Contributor

@Blaisorblade Blaisorblade commented May 13, 2012

asouza: when you add a private attribute to a class, the other class remains as valid as it was, hence it does not need to be recompiled. Removing or changing the public interface, instead, means that the clients have to be recompiled.
It is counterintuitive that adding a public member should lead to recompiling the client class - however I just figured out one corner case where this happens, involving implicit conversion.*
Finally, why does SBT recompiles all transitive dependencies, in your example also Program? When recompiling Person, also its interface might change. Therefore, SBT might need to recompile also Program; whether to do that in a separate step or immediately is mostly a matter of heuristics. The chosen heuristics might change in the future accoring to Mark Harrah - for a discussion see issue #322.

*Suppose that you pimp zipCode on City:

implicit def pimpZipCode(c: City) = new { def zipCode: String = "12345" }

and that you add a usage of this method to Person. If you now add zipCode as a method to City, Person will be recompiled and the access to zipCode will not use pimpZipCode any more.

@asouza

This comment has been minimized.

Copy link

@asouza asouza commented May 14, 2012

The implicit part is ok and recompile the Person class seems to be fair too. But I still don't understand the reason to recompile the Program class, I mean, what will be changed in Person interface when I change the City class? Even with the implicit method problem, the only class that would be affected is the Person class.

@Blaisorblade

This comment has been minimized.

Copy link
Contributor

@Blaisorblade Blaisorblade commented May 14, 2012

Right, I did not address that explicitly. Suppose that Person has a method whose return type is not explicitly specified: type inference will type-check the implementation to decide the return type, which in turn affects the interface. In the example above, suppose that Person has the method
def zipCodes = visitedCities map (_.zipCode),

and suppose that Program uses this method; Program will need to be recompiled if the type of zipCode changes, because calls in bytecode specify the return type of the called method (you can also consider the same situation in Java: there, for equivalent code, you would need to alter type annotations in the source code of Program, hence its bytecode would also change). In the example above we changed the implementation of zipCode (from pimp-my-library to standard method) but not its return type; changing also the return type would however be possible.
If now Program has the method
def zipCodes(p: Person) = p.zipCodes,
even its interface will change when the type of zipCode changes.

However, I have a question for people more experienced: noticing this problem, I started sometimes declaring return types explicitly to reduce such indirect dependencies - that helps, for instance, if the implementation returns ArrayBuffer[String], I declare as return type collection.Seq[CharSequence], and later I change the implementation to return List[CharSequence]. Even with this change, however, sometimes changing the implementation lead to clients being recompiled. Are there other ways in which changing the implementation of a class affects unexpectedly its interface and thus its clients?

@retronym

This comment has been minimized.

Copy link
Member Author

@retronym retronym commented May 14, 2012

It would be best to extract a sample project with a few files and describe the sequence of edits/recompiles that leads to more recompilation than you expect, and open a new ticket.

@Blaisorblade

This comment has been minimized.

Copy link
Contributor

@Blaisorblade Blaisorblade commented May 14, 2012

My question was mostly about knowing better what to expect - do I miss something? If that's not the case, I'll for now assume that I suffered from this same bug, that this bug is fixed in 0.12-Beta2, and try to investigate any future misbehavior.

It would be nice to document these details - I feel I have to somewhat optimize compilation times by reducing dependencies, just like in C++ (with the pImpl C++ pattern, unrelated to pimp-my-library), and I think that's the case for anybody with compile times of minutes.
If you think it makes sense, I could even start creating a page on the SBT wiki with the information I have from these issues (and with questions to fill in).

@asouza

This comment has been minimized.

Copy link

@asouza asouza commented May 15, 2012

I understand all considerations. The implicit case makes sense... This case could be solved, as you said, putting the return type explicitly. But when i just add or remove some method or anything, at least in my opinion, sbt should check only the direct dependencies... This kind of correctness is almost a "clean mode". When i change something, million of classes need to be recompiled.

@Blaisorblade

This comment has been minimized.

Copy link
Contributor

@Blaisorblade Blaisorblade commented May 16, 2012

I will repeat myself: The chosen heuristics [about how much to recompile] might change in the future according to Mark Harrah - for a discussion see issue #322, which is exactly about this complaint of you. I would hence recommend you to simply enable notifications for issue #322 and wait for updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.