Skip to content
This repository has been archived by the owner on Feb 20, 2019. It is now read-only.

Improve performance of implicit search #422

Open
jvican opened this issue May 9, 2016 · 3 comments
Open

Improve performance of implicit search #422

jvican opened this issue May 9, 2016 · 3 comments

Comments

@jvican
Copy link
Member

jvican commented May 9, 2016

These are the times we are looking for an implicit of type T in the implicit search for our test suite (which, btw, is huge):

Type Times
Any 1420
String 1456
Int 1677
Ref 2338

These stats lay the ground for the upcoming improvement in the implicit search that hopefully will address the performance issues that we face right now, especially large compilation times. I think that there's room for improvement and I'm working tirelessly on it.

jvican added a commit to jvican/pickling that referenced this issue May 9, 2016
* Add just a memo that will tell us how many times we are looking for
the pickler/unpickler of a type `T`.
jvican added a commit to jvican/pickling that referenced this issue May 9, 2016
* 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 ;)
@jvican
Copy link
Member Author

jvican commented May 9, 2016

Current:

Type Times
Any 168
String 0
Int 0
Ref 0

I still have to figure out why Any is still being invoked through the macro generation. This hints that the compiler cannot find it in scope. This is probably caused by some test cases not importing AnyPicklerUnpickler from the whole Defaults (we have some tests that cherry-pick the imports) and therefore pickling has to create it from scratch.

My last changes (in my private repo) decrease the compilation time by a half (in my computer the speedup is ~3x). There are still some more improvements coming that will improve preferringAlternativeImplicits.

jvican added a commit to jvican/pickling that referenced this issue May 9, 2016
* 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 ;)
@jvican
Copy link
Member Author

jvican commented May 9, 2016

It also improves #342. I don't own a MBP, but in my Thinkpad x201 which is about 5 years old, the example in the issue consumes ~2s to compile. I think I could cut it even more with the upcoming changes.

@jvican
Copy link
Member Author

jvican commented May 13, 2016

I'm going to put off the next improvements in the implicit search for the next PR, since it's not high priority.

jvican added a commit to jvican/pickling that referenced this issue May 23, 2016
* Add just a memo that will tell us how many times we are looking for
the pickler/unpickler of a type `T`.
jvican added a commit to jvican/pickling that referenced this issue May 23, 2016
* 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 ;)
jvican added a commit to jvican/pickling that referenced this issue May 24, 2016
* Add just a memo that will tell us how many times we are looking for
the pickler/unpickler of a type `T`.
jvican added a commit to jvican/pickling that referenced this issue May 24, 2016
* 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 ;)
jvican added a commit to jvican/pickling that referenced this issue May 24, 2016
* Add just a memo that will tell us how many times we are looking for
the pickler/unpickler of a type `T`.
jvican added a commit to jvican/pickling that referenced this issue May 24, 2016
* 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 ;)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant