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

Some minor optimizations #3159

Merged
merged 5 commits into from
Nov 26, 2019
Merged

Conversation

travisbrown
Copy link
Contributor

This PR replaces some instances of andThen, compose, etc. with function literals. In all of these cases I think the change improves readability—for example this:

def curry[X, Y, Z](f: (X, Y) => Z): X => Y => Z = x => y => f(x, y)
Kleisli[M, E, A](curry(Function.untupled(f)).andThen(R.tabulate))

Is now this:

Kleisli[M, E, A](e => R.tabulate(r => f((e, r))))

It also improves performance, especially in cases like parBitraverse where just rewriting:

f.andThen(P.parallel.apply(_)), g.andThen(P.parallel.apply(_))

To:

a => P.parallel.apply(f(a)), b => P.parallel.apply(g(b))

…avoids multiple allocations and some indirection.

I've included a couple of new benchmarks, and some example results for Scala 2.12 and JVM 8 on my desktop machine:

Benchmark                                       Mode  Cnt         Score        Error  Units
ParTraverseBench.eitherParBitraversePointfree  thrpt   50  20201469.796 ± 143402.778  ops/s
ParTraverseBench.eitherParBitraversePointfull  thrpt   50  24742265.071 ± 133253.733  ops/s
ParTraverseBench.eitherParTraversePointfree    thrpt   50   1150877.660 ±  10162.432  ops/s
ParTraverseBench.eitherParTraversePointfull    thrpt   50   1221809.128 ±   9997.474  ops/s

@codecov-io
Copy link

codecov-io commented Nov 18, 2019

Codecov Report

Merging #3159 into master will not change coverage.
The diff coverage is 92.3%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3159   +/-   ##
=======================================
  Coverage   93.04%   93.04%           
=======================================
  Files         376      376           
  Lines        7374     7374           
  Branches      209      207    -2     
=======================================
  Hits         6861     6861           
  Misses        513      513
Flag Coverage Δ
#scala_version_212 93.34% <92.3%> (-0.03%) ⬇️
#scala_version_213 90.61% <88.46%> (ø) ⬆️
Impacted Files Coverage Δ
core/src/main/scala/cats/data/NonEmptyChain.scala 86.02% <0%> (ø) ⬆️
core/src/main/scala/cats/data/Func.scala 96.42% <0%> (ø) ⬆️
core/src/main/scala/cats/data/Validated.scala 100% <100%> (ø) ⬆️
core/src/main/scala/cats/data/NonEmptyList.scala 98.7% <100%> (ø) ⬆️
core/src/main/scala/cats/data/Nested.scala 95.12% <100%> (ø) ⬆️
core/src/main/scala/cats/data/Kleisli.scala 90.82% <100%> (-0.09%) ⬇️
core/src/main/scala/cats/Parallel.scala 84.84% <100%> (ø) ⬆️
core/src/main/scala/cats/data/IdT.scala 97.72% <100%> (ø) ⬆️
core/src/main/scala/cats/data/Cokleisli.scala 100% <100%> (ø) ⬆️
core/src/main/scala/cats/data/NonEmptySet.scala 97.64% <100%> (ø) ⬆️
... and 2 more

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 8867e97...19a02e1. Read the comment docs.

@travisbrown travisbrown added this to the 2.1.0-RC2 milestone Nov 22, 2019
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find most of them lateral in readability, but tabulate is much nicer, and lateral-but-faster is a win. 👍

@rossabaker rossabaker merged commit 8a958eb into typelevel:master Nov 26, 2019
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.

4 participants