Skip to content

Optimize FreeApplicative.product - #1921

Merged
kailuowang merged 1 commit into
masterfrom
freeap-product
Sep 21, 2017
Merged

Optimize FreeApplicative.product#1921
kailuowang merged 1 commit into
masterfrom
freeap-product

Conversation

@peterneyens

@peterneyens peterneyens commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

Follow up after #1748

Ap now contains two FreeApplicatives (instead of just one before), so we can do some more optimisation as we are currently doing. (The new implementation should also result in the same FreeApplicative as the original implementation).

Currently the result of FreeApplicative's map2 is one of the following two :

Pure(f(a, b))
Ap({ case (a, b) => f(a, b) }, Ap(Ap(Pure(a => b => (a, b)), fa), fb))

With this change it can be one of :

Pure(f(a, b))
Ap(Pure(f(a, _)), fb)
Ap(Pure(f(_, b)), fa)
Ap(Ap(Pure(a => b => f(a, b))), fa), fb)

@codecov-io

codecov-io commented Sep 20, 2017

Copy link
Copy Markdown

Codecov Report

Merging #1921 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1921      +/-   ##
==========================================
+ Coverage   95.54%   95.55%   +<.01%     
==========================================
  Files         248      248              
  Lines        4426     4430       +4     
  Branches      116      117       +1     
==========================================
+ Hits         4229     4233       +4     
  Misses        197      197
Impacted Files Coverage Δ
...ree/src/main/scala/cats/free/FreeApplicative.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 15a5240...5d5f99c. Read the comment docs.

@edmundnoble

Copy link
Copy Markdown
Contributor

This is awesome, it's causing less stack jumps in https://github.com/typelevel/cats/blob/master/free/src/main/scala/cats/free/FreeApplicative.scala#L99 by creating more single-argument functions. 👍 from me.

@edmundnoble

Copy link
Copy Markdown
Contributor

/cc @safareli

@kailuowang
kailuowang merged commit 664cb62 into master Sep 21, 2017
@kailuowang
kailuowang deleted the freeap-product branch October 10, 2017 18:45
@kailuowang kailuowang added this to the 1.0.0-RC1 milestone Oct 13, 2017
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