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

Collection.combineLatest produces stack overflow with many elements #66

Open
palle-k opened this issue Nov 18, 2020 · 5 comments
Open

Comments

@palle-k
Copy link

palle-k commented Nov 18, 2020

When using combineLatest on a collection with around a thousand elements, a stack overflow happens.

minimal example:

var bag: Set<AnyCancellable> = []
(0 ..< 10_000).map {_ in Just(42)}
    .combineLatest()
    .sink { _ in
        bag.removeAll()
    }
    .store(in: &bag)
@freak4pc
Copy link
Member

Is there a practical use case where you combine ten thousand publishers?
Regardless, this uses Combine's underlying publisher so I imagine it is just too much memory for it to withstand. Nothing we can do here.

@palle-k
Copy link
Author

palle-k commented Nov 18, 2020

  1. There are use cases. For example I am downloading many small files, which I then store on disk.
  2. The number of publishers in the example is not necessarily representative of the problems that I am trying to solve, which have much more complex publishers and therefore much more quickly have stack overflow problems. The example with 10000 publishers is just that - a minimal example. When reducing the example to 350 publishers I still managed to produce a stack overflow on a background queue.
  3. The problem is that Collection.combineLatest calls Publisher.combineLatest (note: this is not Combine's combineLatest - it is an overload defined by CombineExt), which recursively calls itself for every element of the collection of publishers. This is not a Combine issue and neither is it a problem with general memory usage.

@freak4pc
Copy link
Member

Happy to accept a PR if you have any idea to optimize it for large collections.

@palle-k
Copy link
Author

palle-k commented Nov 18, 2020

I will see whether I can do so - but this will happen Monday the earliest.

This issue should IMO be left open unless properly resolved.

@freak4pc
Copy link
Member

I would honestly prefer an open PR and not an open issue.
If an issue is a wide-spread bug, I agree, but if it only affects a very narrow use case, it's usually easy to discuss around a PR. Regardless, will reopen this for now and we can see what's the status next week. Thanks!

@freak4pc freak4pc reopened this Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants