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

Fix #712: add Iterant.uncons operation #826

Merged
merged 4 commits into from Feb 10, 2019

Conversation

Projects
None yet
2 participants
@oleg-py
Copy link
Collaborator

commented Feb 2, 2019

  • Both Resource and Iterant versions (latter could be optimized, but now it's just convenience to avoid fromResource.
  • Single element only. Batch versions can be made later.

@alexandru should I add separate tests to check how resource alloc works, or laws are enough on our setup?

oleg-py added some commits Feb 2, 2019

@alexandru

This comment has been minimized.

Copy link
Member

commented Feb 5, 2019

@oleg-py this is very cool 🎉

What I'm wondering though is how useful it is, given that usage (of Resource at least) implies evaluation via bracket, which is not tail recursive, so I'm wondering if it's feasible to process an entire stream (that may be really big or infinite) via unconsR without memory blowing up.

How does it behave if it's a pure (no resources to close), but infinite stream. Can you do a quick test for yourself?

Otherwise Resource usage is fine, it communicates the result better, I don't think we need the Iterant version.

@oleg-py

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 5, 2019

@alexandru you are correct about memory usage. Seems that on every .pure of Resource, a lambda is allocated and is never released, so it is not suited for processing infinite streams. I rewrote it to use Iterant instead - with Iterant, all allocated nodes are eventually getting de-allocated (except for degenerate case of infinitely nested Scopes).

I'm very keen on preserving the Iterant version. I assume it's going to be used to implement custom operators, e.g. grouping in pairs:

def pairs(i: Iterant[F, A]): Iterant[F, (A, A)] = i.uncons.flatMap { case (opt, tail) =>
  tail.uncons.flatMap { case (opt2, tail2) =>
    (opt1, opt2).tupled.fold(Iterant.empty[F, A])(_ +: pairs(tail2))
  }
}

where you want to produce Iterant back as a result eventually. Resource version is better for folds, but Iterant version is better for transforms.

I've removed Resource version now. Can re-add it with a note that it's not safe for recursing on infinite streams.

@alexandru

This comment has been minimized.

Copy link
Member

commented Feb 10, 2019

Thanks for checking. The Iterant version is great if it works for pure and infinite streams.

Lets not add the Resource version for now.

@alexandru alexandru merged commit 48e5195 into monix:master Feb 10, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.