In rankImplicits, before we attempt to fully typecheck the pending candidate implicit, we first attempt to partially instantiate type variables in both the candidate and the target type and check for compatibility. If the compatibility check fails we can immediately prune the the candidate without having to fully typecheck it.

In the kinds of implicit searches typical of the inductive style found in shapeless and related libraries this can result in a drastic reduction in the search space and a corresponding reduction in compile times.

This commit is much simpler, more generally applicable, and less invasive than my earlier work on inductive implicits in #6481 which was doing similar pruning almost by accident. It turns out that almost all of the speedups in that earlier PR were due to the pruning rather than to any special casing of inductive patterns.

The compilation benchmark (a shapeless-style "select element by type from a large HList") from that PR is carried over here (in test/induction) and shows the same performance improvements,

As an added bonus users of shapeless and shapeless-based libraries which use shapeless's Lazy type will see benefits immediately without needing to wait for and port to byname implicit arguments.

Trying this change with your project

To test this with your shapeless-using project checkout out this branch and fire up the sbt repl then clean, compile and dist/mkPack. This should give you a Scala distribution in <path to scala checkout>/build/pack.

If you now fire up an sbt repl on the project you want to build with this compiler you can then set the Scala version at the prompt via ++2.12.5=<path to scala checkout>/build/pack. You should now be able to compile and test:compile as if with 2.12.5, but with this optimization in place.

This comment has been minimized.

edited

For reference, I just tried this change in Bloop. The frontend of bloop is the biggest module in our build, and it uses Shapeless via caseapp. Our compile times have been unusually slow because of Lazy. I'm seeing a 2x speedups in cold compilation, which is a great number. We're not even big users of Shapeless (only one file using Shapeless in the whole repository), so I would expect Shapeless-based codebases (many in our ecosystem) to benefit from this change.

my work project, (~500 files, 30k LOC) compile in main and all test configurations

2.12.5: 167s

this: 146s (but sbt-assembly failed due to duplicate classfile entries, seems there is an sbt bug about pruning the scala-library.jar when using a custom scala dist)

just compile: 49s vs 45s (a small speedup)

All these projects are heavy users of shapeless generic derivation. However, I am using "semiauto" style, i.e. def gen is not implicit, and therefore not recursive. It is more boilerplate, but I feel it addresses all the problems that were highlighted in export-hook whilst being much faster to compile (I use the @scalaz.deriving annotation to generate the boilerplate).

I suspect semiauto is equivalent to the automated polymorphic pruning. Perhaps it would be feasible to disable semiauto and just derive the top level of the ADT, but I don't have time to do that refactor at the moment. My work project also has some scanamo and pureconfig uses, which are doing full auto derivation at the point of use, so that's where I guess the benefits are coming from.

sbt outputs this which gives me reason to believe it is working as expected:

> ++2.12.5=/home/fommil/Projects/scala/build/pack!
[info] Using Scala home /home/fommil/Projects/scala/build/pack with actual version 2.12.7-20180430-184100-af4ffa8
[info] Forcing Scala version to 2.12.5 on all projects.

This comment has been minimized.

I suspect semiauto is equivalent to the automated polymorphic pruning. Perhaps it would be feasible to disable semiauto and just derive the top level of the ADT

That would be a very useful comparison to make: semiauto style trades more boilerplate for lower compilation times, so if with this change fully automatic derivation is on a par with semiauto wrt compile times that's a big win.

This comment has been minimized.

That would be a very useful comparison to make: semiauto style trades more boilerplate for lower compilation times, so if with this change fully automatic derivation is on a par with semiauto wrt compile times that's a big win.

Note that there's still value in having semiauto over fully automatic derivation: runtime performance and code size. Semiauto is more boilerplate-y but allows programmers to control better where the instances live, and even ensure there's only one instance per type in the codebase.

I don't know how to explain this. I would really like to see @stephennancekivell redo the benchmarks. It would be very interesting if full auto was somehow faster than semi-auto with this patch applied! Perhaps because it finds things in the import scope and never has to check companions and nested package objects.

This comment has been minimized.

edited

I updated those json codec deriving benchmarks, but unfortunately the results aren’t very promising. Maybe my benchmarks are more impacted by the case class to Hlist macro, than the amount of implicits looked up.

In rankImplicits, before we attempt to fully typecheck the pending
candidate implicit, we first attempt to partially instantiate type
variables in both the candidate and the target type and check for
compatibility. If the compatibility check fails we can immediately prune
the the candidate without having to fully typecheck it.
In the kinds of implicit searches typical of the inductive style found
in shapeless and related libraries this can result in a drastic
reduction in the search space and a corresponding reduction in compile
times.
This commit is much simpler, more generally applicable, and less
invasive than my earlier work on inductive implicits in,
#6481
which was doing similar pruning almost by accident. It turns out that
almost all of the speedups in that earlier PR were due to the pruning
rather than to any special casing of inductive patterns.
The compilation benchmark (a shapeless-style "select element by type
from a large HList") from that PR is carried over here (in
test/induction) and shows the same performance improvements,
(1) (2)
HList Size
50 4 3
100 7 3
150 15 4
200 28 4
250 48 5
300 81 6
350 126 8
400 189 11
450 322 13
500 405 16
Compile time in seconds
As an added bonus users of shapeless and shapeless based libraries which
use shapeless's Lazy type will see benefits immediately without needing
to wait for and port to byname implicit arguments.

This comment has been minimized.

@SethTisue the failures here look pretty random and mostly IO and environment related. I find it fairly hard to believe that they're related to this PR. What's the current expected state of the 2.12.x community build?

This comment has been minimized.

akka-more: looks like a transient failure to resolve a dependency, should go away on re-run

http4s, breeze, eff, classutil: idk what these are, I have not seen these in any other runs, so I think you have to suspect your PR first, I'm afraid. but also, just try re-running the whole thing (it shouldn't take 13 hours this time since dbuild will have previous results cached)

This comment has been minimized.

@SethTisue the classutil failure is reproducible, but unrelated to this PR ... one of the tests is expecting a non-None result from scala.util.Properties.releaseVersion. Do you have something similar to the below for the main 2.12.x community build? Or is there something missing in the configuration for this this build?

This comment has been minimized.

The failure in specs2-scalaz is a bit of a weird one. It turns out that the backport of #6612 is the culprit. It seems that the the failing line in TaskMatchers was relying on side-effects in the unseen and now suppressed TypeDiagnostics.findReqMsg.

#6612 is present in 2.13.0-M4 which spec2-scalaz builds with, so what's happening there? It turns out that the problem has been worked around, I guess more or less incidentally as part of the process of updating for the new collections,

Applying the same change to specs2 for the 2.12.x build also fixes the problem with the backport of #6612 here too.

@SethTisue I'd like to push on with this community build ... would you be happy to fork specs2 with the above change? Is there some way it can be done without disrupting the normal 2.12.x community build?

This comment has been minimized.

I noticed one of our larger Scala projects fails to build using this patch (assuming I correctly built Scala using this branch).

The code that fails is basically this:

implicitdeftoOption[T](v: T):Option[T] =Option(v)
vala:Int=123valb:Option[Long] = a // Works under 2.12.6 but not with the implicit-poly-prune-2.12.x PR

I'm not really sure if this was intended to work in the first place but this pattern
showed up in a large Scala project we have. I'll probably remove the implicit from
our code and make all the conversions explicit since that is not a very good implicit
to have in the first place. However I'm not sure if the PR intended to change this
behavior.

This comment has been minimized.

is that [Throwable] has to be added to help type inference later on with ^^. Is that right? This use of ^^ (or adapt) is generally localized, meaning it is only used when defining matchers, not using them. If on the other hand every time people wanted to write be_-\/(haveClass[T]) they would have to write be_-\/[Throwable](haveClass[T]) that would be breaking a lot more code.

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.