This repository has been archived by the owner on Feb 20, 2019. It is now read-only.
Improve implicit search drastically #424
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm still polishing a little bit this PR and make it pass! |
|
FYI you should merge with your test fixes. That was passing os I merged. Looks like I have a bunch of merge conflicts to resovle in my branch too :) |
jvican
force-pushed
the
improve-implicit-search
branch
2 times, most recently
from
cb9480b
to
804178e
Compare
* Improve debugging of the algorithm and put some comments. * Gather `GenPicklers` and `GenUnpicklers` in the same file, remove unused trait. Update `AllPicklerUnpicklers` accordingly.
* Reuse `AlgorithmLogger` to report errors * Do some basic cleanup and lay the ground for future work on improving the `preferringAlternativeImplicits`.
* We don't need `PicklingMacros` when we can use the more explicit `Pickler.generate` and family. * Rename `Macros` to `PicklingMacros`.
* Add just a memo that will tell us how many times we are looking for the pickler/unpickler of a type `T`.
* Change all the implicit definitions to return `AbstractPicklerUnpickler` instead of `Pickler[T] with Unpickler[T]`. The former one has more precedence than the latter one. This avoids to call the generator macro EVERY TIME we ask for an implicit pickler/unpickler. * Make sure that the `LowPriorityImplicits` are placed in the right location of the `Defaults` trait. * This addresses a big deal of scala#422 although more changes will come, especially affecting `preferringAlternativeImplicits`. * Do some minor cleanup and formatting changes (trying out scalafmt) in the project. * Remove explicit registerings of picklers that were not necessary because of `AutoRegister`. * Register myself to the TODOs that are concerned with the creation of runtime picklers/unpicklers. @jsuereth, I've stolen you some TODOs ;)
* Keep around the infrastructure used for debugging the implicit search because we'll come back to improve it further soon.
* Import `HasCompat` to expose the 2.10 macros stub and move around the implicit class hack to make `pt` compile. This is quite flaky, we should use something like `macro-compat`. * Change type signature of picklers/unpicklers in wikigraph to avoid clash.
jvican
force-pushed
the
improve-implicit-search
branch
from
804178e
to
d516a68
Compare
|
This is ready for review 👍 I've fixed the compatibility issues with 2.10. |
|
This is also ready for merge @jsuereth |
| @@ -353,45 +354,6 @@ abstract class Macro extends RichTypes { self => | |||
| def syntheticPicklerUnpicklerQualifiedName(tpe: Type): TypeName = syntheticBaseQualifiedName(tpe) + syntheticPicklerUnpicklerSuffix() | |||
| def syntheticPicklerUnpicklerSuffix(): String = "PicklerUnpickler" | |||
|
|
|||
| def preferringAlternativeImplicits(body: => Tree): Tree = { | |||
|
Closed and reopened to force Travis to execute the tests. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Disable logging of pickler generation
because we'll come back to improve it further soon.
Add big improvement in the implicit search
AbstractPicklerUnpicklerinstead ofPickler[T] with Unpickler[T].The former one has more precedence than the latter one. This avoids to
call the generator macro EVERY TIME we ask for an implicit
pickler/unpickler.
LowPriorityImplicitsare placed in the rightlocation of the
Defaultstrait.especially affecting
preferringAlternativeImplicits.the project.
because of
AutoRegister.runtime picklers/unpicklers. @jsuereth, I've stolen you some TODOs ;)
Add benchmark utility that helps address #422
pickler/unpickler of a type
T.Remove cumbersome
PicklingMacrosdefinitionPicklingMacroswhen we can use the more explicitPickler.generateand family.MacrostoPicklingMacros.Move
preferringAlternativeImplicitsto MacrosAlgorithmLoggerto report errorspreferringAlternativeImplicits.First step towards better implicit search
GenPicklersandGenUnpicklersin the same file, removeunused trait. Update
AllPicklerUnpicklersaccordingly.