Optimize traverse #3283
Merged
Optimize traverse #3283
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
Codecov Report
@@ Coverage Diff @@
## master #3283 +/- ##
==========================================
- Coverage 93.14% 93.14% -0.01%
==========================================
Files 378 378
Lines 7576 7575 -1
Branches 203 194 -9
==========================================
- Hits 7057 7056 -1
Misses 519 519
Continue to review full report at Codecov.
|
kailuowang
approved these changes
Feb 3, 2020
djspiewak
approved these changes
Feb 3, 2020
There was a problem hiding this comment.
At first glance, seems like we could potentially shave even more off of this with some dirtier internal tricks. This is a great start though.
Performance improvements on generic typeclass operations generally don't particularly interest me, since they should never be in the hot path anyway, but faster is always better than slower, regardless of the context.
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.
tl;dr: Traversing a
ListorVectoris probably the most common operation people do with this library, and the current implementations for these types have some room for optimization, with the changes in this PR giving up to 20% more throughput forListand 201% forVector.I've put together a benchmark that compares the current
traverseforListwith two new implementations:The first uses the standard library's
foldRightwith an explicitEvalaccumulator, instead of theEval-basedfoldRightonFoldable. The second effectively just inlines the call the Cat'sfoldRightin the current implementation.Both of these seem substantially faster than the current implementation when traversing with
Right(_)(results shown for list sizes 101, 102, 103, and 104; higher numbers are better; all results are shown for Scala 2.13, but 2.12 is similar):The
loopimplementation also allocates less:The results for a more complex parsing operation in
ValidatedNelare similar.I've done a similar comparison for
Vector, but with an additional new candidate:I've also included implementations of all three new approaches for
Vectorthat accumulate the result in aListand then convert at the end.Again the
loopimplementation is fastest (but the version that accumulates in a list, not the one that builds a vector directly).I've made these changes for
List,Vector, and 2.13'sArraySeq, but not forChain,Stream, orLazyList.