Minor performance improvement in Free #776
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
|
Wow great work! 👍 on merging.
…On Wed, Nov 23, 2016 at 10:10 PM Michael Pilquist ***@***.***> wrote:
This PR improves performance of Free by eagerly applying Bind(Pure(a), f)
patterns in Free#step. We used to do this but it had the unfortunate side
effect of evaluating suspends, because suspend is (still) implemented as pure(())
>> fa. This PR takes a conservative approach to fixing this issue by
introducing a new flag on the Pure constructor, indicating whether or not
eager evaluation is supported. Free.pure sets the flag to true whereas
Free.suspend sets the flag to false.
The Free benchmarks show 18% - 42% increase:
a0aef44
<a0aef44>
(head of series/1.0)
[info] Benchmark Mode Cnt Score Error Units
[info] FreeBenchmark.nestedFlatMaps thrpt 10 11.023 ± 1.311 ops/s
[info] FreeBenchmark.nestedMaps thrpt 10 7.292 ± 1.810 ops/s
fa9aa99
<fa9aa99>
(this PR)
info] Benchmark Mode Cnt Score Error Units
[info] FreeBenchmark.nestedFlatMaps thrpt 10 13.660 ± 1.745 ops/s
[info] FreeBenchmark.nestedMaps thrpt 10 10.412 ± 1.739 ops/s
The Stream benchmarks show a wider range of improvements -- the worse case
is about equal (runLog_102400) and the best case shows a 330% improvement
(awaitPull_256).
[info] Benchmark Mode Cnt Score Error Units
[info] StreamBenchmark.awaitPull_256 thrpt 10 57.634 ± 8.218 ops/s
[info] StreamBenchmark.eval_102400 thrpt 10 0.616 ± 0.025 ops/s
[info] StreamBenchmark.leftAssocConcat_102400 thrpt 10 3.261 ± 0.448 ops/s
[info] StreamBenchmark.leftAssocFlatMap_102400 thrpt 10 9.509 ± 0.501 ops/s
[info] StreamBenchmark.rightAssocConcat_102400 thrpt 10 3.171 ± 0.409 ops/s
[info] StreamBenchmark.rightAssocFlatMap_102400 thrpt 10 8.579 ± 0.496 ops/s
[info] StreamBenchmark.runLog_102400 thrpt 10 770.429 ± 28.606 ops/s
fa9aa99
<fa9aa99>
(this PR)
[info] Benchmark Mode Cnt Score Error Units
[info] StreamBenchmark.awaitPull_256 thrpt 10 188.070 ± 16.577 ops/s
[info] StreamBenchmark.eval_102400 thrpt 10 1.443 ± 0.317 ops/s
[info] StreamBenchmark.leftAssocConcat_102400 thrpt 10 5.101 ± 0.323 ops/s
[info] StreamBenchmark.leftAssocFlatMap_102400 thrpt 10 12.276 ± 1.137 ops/s
[info] StreamBenchmark.rightAssocConcat_102400 thrpt 10 5.069 ± 0.568 ops/s
[info] StreamBenchmark.rightAssocFlatMap_102400 thrpt 10 12.130 ± 1.069 ops/s
[info] StreamBenchmark.runLog_102400 thrpt 10 767.122 ± 14.423 ops/s
I think we should merge this in order to establish a new baseline for
performance, and then do the more extensive Free refactorings, like map
fusion with type aligned sequences, after this version.
------------------------------
You can view, comment on, or merge this pull request online at:
#776
Commit Summary
- Added start of benchmark of Free
- Eagerly evaluate Bind(Pure(..), ..) patterns in Free for everything
except Free.suspend
File Changes
- *A* benchmark/src/main/scala/fs2/benchmark/FreeBenchmark.scala
<https://github.com/functional-streams-for-scala/fs2/pull/776/files#diff-0>
(24)
- *M* core/shared/src/main/scala/fs2/util/Free.scala
<https://github.com/functional-streams-for-scala/fs2/pull/776/files#diff-1>
(15)
Patch Links:
- https://github.com/functional-streams-for-scala/fs2/pull/776.patch
- https://github.com/functional-streams-for-scala/fs2/pull/776.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#776>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAArQieofsH0IjRicCzkbBMZSr7_ALYZks5rBQA4gaJpZM4K7PUf>
.
|
|
Benchmarks for the 2 minor commits today -- some minor improvements to raw Free benchmarks but about the same on Stream benchmarks: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR improves performance of Free by eagerly applying
Bind(Pure(a), f)patterns inFree#step. We used to do this but it had the unfortunate side effect of evaluating suspends, because suspend is (still) implemented aspure(()) >> fa. This PR takes a conservative approach to fixing this issue by introducing a new flag on thePureconstructor, indicating whether or not eager evaluation is supported.Free.puresets the flag to true whereasFree.suspendsets the flag to false.The Free benchmarks show 18% - 42% increase:
a0aef44 (head of series/1.0)
fa9aa99 (this PR)
The Stream benchmarks show a wider range of improvements -- the worse case is about equal (
runLog_102400) and the best case shows a 330% improvement (awaitPull_256).a0aef44 (head of series/1.0)
fa9aa99 (this PR)
I think we should merge this in order to establish a new baseline for performance, and then do the more extensive Free refactorings, like map fusion with type aligned sequences, after this version.