Fix traverse_ for List and Vector to be stack safe. #3960
Conversation
|
take a look @armanbilge |
johnynek
changed the title
|
Cool!! So, if I'm understanding correctly, this doesn't actually make it stack safe (in the theoretical sense) but more so in the practical sense, that it's effectively impossible to blow the stack due since its depth is now the logarithm of the size of the input? |
|
So, I think we generally say that going Note, the sun does not have enough energy to enumerate all numbers to 2^200, so 2^1000 even is an unimaginably large number. |
| @@ -32,6 +32,12 @@ abstract class TraverseSuite[F[_]: Traverse](name: String)(implicit ArbFInt: Arb | |||
| } | |||
| } | |||
|
|
|||
| test(s"Traverse[$name].traverseMatches_ with Option") { | |||
| forAll { (fa: F[Int], fn: Int => Option[Int]) => | |||
| assert(Applicative[Option].void(fa.traverse(fn)) == fa.traverse_(fn)) | |||
There was a problem hiding this comment.
there is no law on this, and probably should be, but requires source and binary changes to laws.
There was a problem hiding this comment.
FYI laws can be introduced without breaking either source or binary compatibility
|
Maybe we need |
|
I don't think so. The point of traverseViaChain is to build back up a sequence from a Tree structure. But in the case of traverse_ we don't build a resulting sequence: we just return the Applicative |
|
Ahh right it could be just |
| @@ -32,6 +32,12 @@ abstract class TraverseSuite[F[_]: Traverse](name: String)(implicit ArbFInt: Arb | |||
| } | |||
| } | |||
|
|
|||
| test(s"Traverse[$name].traverseMatches_ with Option") { | |||
There was a problem hiding this comment.
did you mean to say traverse_ matches traverse with Option?
There was a problem hiding this comment.
good catch, thanks. Will you take a look again?
There was a problem hiding this comment.
This looks really good, but while you're in here… I would like to interest you in #3790. What you're touching right here is the bottleneck for any and all par operators, and the Eval wrapping is the most substantial cost difference between this generic implementation and a hand-optimized and specialized implementation.
|
@djspiewak I'll reply on that issue with some thoughts. I agree we are in a suboptimal situation at the moment. |
This fixes traverse_ for List and Vector to avoid stack overflow errors.
This is a partial fix for #3947