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

Enable name hashing incremental compilation by default. #1546

Merged
merged 1 commit into from Aug 22, 2014

Conversation

gkossakowski
Copy link
Contributor

This commit changes the default value of IncOptions.nameHashing to be set to true. It means, the improved incremental compilation algorithm known as "name hashing" will be enabled by default.

In order to disable it, users should add this to their sbt configuration:

  incOptions := incOptions.value.withNameHashing(false)

Number of tests has been cleaned up as part of this change. All tests that were marked as name hashing specific are removed. The list includes:

  • constants-name-hashing
  • import-class-name-hashing
  • java-static-name-hashing
  • macro-name-hashing
  • struct-name-hashing

We'll keep just regular version of those tests. The tests will just exercise the default algorithm: name hashing. This is the first step towards phasing out of the old incremental compilation algorithm.

Apart from that, a few tests changed its status due to enabling name hashing algorithm.

The constants test has been marked pending due to issue described in #1543.

The import-class test has been marked as passing because name hashing tracks dependencies introduced by import statements correctly, now.

The macro test has been marked as pending due to issue described in #1544.

The struct test has been marked as pending due to issue described in #1545.

The java-static has been slightly modified to exercise just static field and not run into the same issue as with constants test.

There are no other known issues related to name hashing so we conclude that name hashing is ready to be enabled by default for all sbt, Scala IDE and zinc users.

This commit changes the default value of `IncOptions.nameHashing` to be
set to true. It means, the improved incremental compilation algorithm
known as "name hashing" will be enabled by default.

In order to disable it, users should add this to their sbt configuration:

  incOptions := incOptions.value.withNameHashing(false)

Number of tests has been cleaned up as part of this change. All tests
that were marked as name hashing specific are removed. The list includes:

  * constants-name-hashing
  * import-class-name-hashing
  * java-static-name-hashing
  * macro-name-hashing
  * struct-name-hashing

We'll keep just regular version of those tests. The tests will just
exercise the default algorithm: name hashing. This is the first step
towards phasing out of the old incremental compilation algorithm.

Apart from that, a few tests changed its status due to enabling name
hashing algorithm.

The `constants` test has been marked pending due to issue described in
sbt#1543.

The `import-class` test has been marked as passing because name hashing
tracks dependencies introduced by import statements correctly, now.

The `macro` test has been marked as pending due to issue described in
sbt#1544.

The `struct` test has been marked as pending due to issue described in
sbt#1545.

The `java-static` has been slightly modified to exercise just static field
and not run into the same issue as with `constants` test.

There are no other known issues related to name hashing so we conclude
that name hashing is ready to be shipped to all sbt, Scala IDE and zinc
users.
@gkossakowski
Copy link
Contributor Author

Review by @eed3si9n

I hope we can still merge this into 0.13.6. However, I won't mind merging it into some other release if you decide I'm late to the game.

@typesafe-tools
Copy link

Can one of the admins verify this patch?

eed3si9n added a commit that referenced this pull request Aug 22, 2014
Enable name hashing incremental compilation by default.
@eed3si9n eed3si9n merged commit c69b006 into sbt:0.13 Aug 22, 2014
@eed3si9n
Copy link
Member

Let's do it!

@gkossakowski gkossakowski deleted the nameHashingOnByDefault branch August 22, 2014 01:30
@eed3si9n
Copy link
Member

@gkossakowski I hit the merge button too quickly.
I am now seeing errors in other builds that I suspect is related to this merge. See https://travis-ci.org/sbt/sbt/builds/33265884

[info] java.lang.UnsupportedOperationException: The `++` operation is not supported for relations with different values of `nameHashing` flag.
[info]  at sbt.inc.MRelationsDefaultImpl.$plus$plus(Relations.scala:364)
[info]  at sbt.inc.MAnalysis.$plus$plus(Analysis.scala:141)
[info]  at Build$$anonfun$root$8$$anonfun$apply$1.apply(Build.scala:7)
[info]  at Build$$anonfun$root$8$$anonfun$apply$1.apply(Build.scala:7)
[info]  at scala.collection.LinearSeqOptimized$class.foldLeft(LinearSeqOptimized.scala:111)
[info]  at scala.collection.immutable.List.foldLeft(List.scala:84)
[info]  at scala.collection.TraversableOnce$class.$div$colon(TraversableOnce.scala:138)
[info]  at scala.collection.AbstractTraversable.$div$colon(Traversable.scala:105)
[info]  at Build$$anonfun$root$8.apply(Build.scala:7)
[info]  at Build$$anonfun$root$8.apply(Build.scala:7)
[info]  at scala.Function1$$anonfun$compose$1.apply(Function1.scala:47)
[info]  at sbt.$tilde$greater$$anonfun$$u2219$1.apply(TypeFunctions.scala:40)
[info]  at sbt.std.Transform$$anon$4.work(System.scala:63)
[info]  at sbt.Execute$$anonfun$submit$1$$anonfun$apply$1.apply(Execute.scala:226)
[info]  at sbt.Execute$$anonfun$submit$1$$anonfun$apply$1.apply(Execute.scala:226)
[info]  at sbt.ErrorHandling$.wideConvert(ErrorHandling.scala:17)
[info]  at sbt.Execute.work(Execute.scala:235)
[info]  at sbt.Execute$$anonfun$submit$1.apply(Execute.scala:226)
[info]  at sbt.Execute$$anonfun$submit$1.apply(Execute.scala:226)
[info]  at sbt.ConcurrentRestrictions$$anon$4$$anonfun$1.apply(ConcurrentRestrictions.scala:159)
[info]  at sbt.CompletionService$$anon$2.call(CompletionService.scala:28)
[info]  at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
[info]  at java.util.concurrent.FutureTask.run(FutureTask.java:166)
[info]  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
[info]  at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
[info]  at java.util.concurrent.FutureTask.run(FutureTask.java:166)
[info]  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
[info]  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
[info]  at java.lang.Thread.run(Thread.java:701)
[info] java.lang.AssertionError: assertion failed: Name hashing and Ant-style cannot be enabled at the same time.
[info]  at scala.Predef$.assert(Predef.scala:179)
[info]  at sbt.inc.IncOptions.<init>(IncOptions.scala:96)
[info]  at sbt.inc.IncOptions.withAntStyle(IncOptions.scala:144)
[info]  at $b6aa51793996a2cedafc$$anonfun$$sbtdef$1.apply(/tmp/sbt_fdbbb777/inc-ant-style/build.sbt:3)
[info]  at $b6aa51793996a2cedafc$$anonfun$$sbtdef$1.apply(/tmp/sbt_fdbbb777/inc-ant-style/build.sbt:3)
[info]  at scala.Function1$$anonfun$compose$1.apply(Function1.scala:47)
[info]  at sbt.$tilde$greater$$anonfun$$u2219$1.apply(TypeFunctions.scala:40)
[info]  at sbt.std.Transform$$anon$4.work(System.scala:63)
[info]  at sbt.Execute$$anonfun$submit$1$$anonfun$apply$1.apply(Execute.scala:226)
[info]  at sbt.Execute$$anonfun$submit$1$$anonfun$apply$1.apply(Execute.scala:226)
[info]  at sbt.ErrorHandling$.wideConvert(ErrorHandling.scala:17)
[info]  at sbt.Execute.work(Execute.scala:235)
[info]  at sbt.Execute$$anonfun$submit$1.apply(Execute.scala:226)
[info]  at sbt.Execute$$anonfun$submit$1.apply(Execute.scala:226)
[info]  at sbt.ConcurrentRestrictions$$anon$4$$anonfun$1.apply(ConcurrentRestrictions.scala:159)
[info]  at sbt.CompletionService$$anon$2.call(CompletionService.scala:28)
[info]  at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
[info]  at java.util.concurrent.FutureTask.run(FutureTask.java:166)
[info]  at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
[info]  at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
[info]  at java.util.concurrent.FutureTask.run(FutureTask.java:166)
[info]  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
[info]  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
[info]  at java.lang.Thread.run(Thread.java:701)

eed3si9n added a commit that referenced this pull request Aug 22, 2014
Name hashing is now turned on by default, so I’m changing the value for
inc.Relations.empty, so inc.Analysis.empty functions as expected when
it’s joined with name hashing analyses.
@gkossakowski
Copy link
Contributor Author

Sorry for missing those tests. I thought that only source-dependencies would be affected by enabling name hashing.

eed3si9n added a commit that referenced this pull request Aug 23, 2014
Fixes #1550/#1546. Name hashing change to fix the build
eed3si9n added a commit that referenced this pull request Aug 23, 2014
eed3si9n added a commit that referenced this pull request Aug 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants