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

Do not use eval for known RefType instances #332

Merged
merged 2 commits into from Sep 27, 2017

Conversation

fthomas
Copy link
Owner

@fthomas fthomas commented Sep 27, 2017

This avoids to call eval for known RefType instances in
RefineMacro.impl. This is similar to #330 but more general
since it is an optimization for all macros in the library and
not only for autoRefine{V,T}.

The performance improvements are similar to that of #330:

[info] Result "eu.timepit.refined.benchmark.RefineMacroBenchmark.autoRefineV_PosInt":
[info]   0.408 ±(99.9%) 0.003 s/op [Average]
[info]   (min, avg, max) = (0.387, 0.408, 0.442), stdev = 0.012
[info]   CI (99.9%): [0.405, 0.411] (assumes normal distribution)
[info] # Run complete. Total time: 00:09:11
[info] Benchmark                                Mode  Cnt  Score   Error Units
[info] RefineMacroBenchmark.autoRefineV_PosInt  avgt  200  0.408 ± 0.003 s/op

This avoids to call `eval` for known `RefType` instances in
`RefineMacro.impl`. This is similar to #330 but more general
since it is an optimization for all macros in the library and
not only for `autoRefine{V,T}`.

The performance improvements are similar to that of #330:
```
[info] Result "eu.timepit.refined.benchmark.RefineMacroBenchmark.autoRefineV_PosInt":
[info]   0.408 ±(99.9%) 0.003 s/op [Average]
[info]   (min, avg, max) = (0.387, 0.408, 0.442), stdev = 0.012
[info]   CI (99.9%): [0.405, 0.411] (assumes normal distribution)
[info] # Run complete. Total time: 00:09:11
[info] Benchmark                                Mode  Cnt  Score   Error Units
[info] RefineMacroBenchmark.autoRefineV_PosInt  avgt  200  0.408 ± 0.003 s/op

```
@fthomas
Copy link
Owner Author

fthomas commented Sep 27, 2017

Another advantage over #330 is that this does not change any public API.

@codecov
Copy link

codecov bot commented Sep 27, 2017

Codecov Report

Merging #332 into master will decrease coverage by 2.78%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #332      +/-   ##
==========================================
- Coverage   99.01%   96.23%   -2.79%     
==========================================
  Files          38       38              
  Lines         408      425      +17     
  Branches        4        6       +2     
==========================================
+ Hits          404      409       +5     
- Misses          4       16      +12
Impacted Files Coverage Δ
.../scala/eu/timepit/refined/macros/RefineMacro.scala 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71591b3...8bb477e. Read the comment docs.

@fthomas fthomas merged commit 07379a2 into master Sep 27, 2017
@fthomas fthomas deleted the topic/avoid-eval-for-reftype branch September 27, 2017 21:04
fthomas added a commit that referenced this pull request Sep 28, 2017
This is the same as #332 but for `InferMacro`.
fthomas added a commit that referenced this pull request Sep 29, 2017
This avoids to call `eval` for a few `Validate` instances in
`RefinedMacro.impl`. This is similar in spirit to #332.

For the `autoRefineV_PosInt` benchmark we get a decrease in compilation
time of ~56% (or ~67% prior to #332):
```
[info] Result "eu.timepit.refined.benchmark.RefineMacroBenchmark.autoRefineV_PosInt":
[info]   0.181 ±(99.9%) 0.001 s/op [Average]
[info]   (min, avg, max) = (0.173, 0.181, 0.234), stdev = 0.006
[info]   CI (99.9%): [0.180, 0.182] (assumes normal distribution)
[info] # Run complete. Total time: 00:08:03
[info] Benchmark                                Mode  Cnt  Score   Error Units
[info] RefineMacroBenchmark.autoRefineV_PosInt  avgt  200  0.181 ± 0.001 s/op
```
fthomas added a commit that referenced this pull request Sep 29, 2017
This avoids to call `eval` for a few `Validate` instances in
`RefinedMacro.impl`. This is similar in spirit to #332.

For the `autoRefineV_PosInt` benchmark we get a decrease in compilation
time of ~56% (or ~67% prior to #332):
```
[info] Result "eu.timepit.refined.benchmark.RefineMacroBenchmark.autoRefineV_PosInt":
[info]   0.181 ±(99.9%) 0.001 s/op [Average]
[info]   (min, avg, max) = (0.173, 0.181, 0.234), stdev = 0.006
[info]   CI (99.9%): [0.180, 0.182] (assumes normal distribution)
[info] # Run complete. Total time: 00:08:03
[info] Benchmark                                Mode  Cnt  Score   Error Units
[info] RefineMacroBenchmark.autoRefineV_PosInt  avgt  200  0.181 ± 0.001 s/op
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant