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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Observable concatenation (++) stack safe #391

Merged
merged 5 commits into from Aug 1, 2017

Conversation

Projects
None yet
1 participant
@alexandru
Member

alexandru commented Aug 1, 2017

In version 2.x.x this use-case isn't supported:

def range(from: Int, until: Int): Observable[Int] = 
  Observable.defer {
    Observable.now(from) ++ (
      if (from + 1 < until) loop(from + 1, until)
      else Observable.empty
    )
  }

The reason is that ++ is not memory (stack) safe. So up until now I've been redirecting users to Observable.tailRecM instead.

The reason has to do with historical clouds 馃槂 The ++ operator has been based on flatMap, but flatMap is a very hard to implement operator for Observable and I could not find a way to use constant memory in recursive loops due to the push-based protocol.

Well, apparently ++ admits an easy solution and now the use-case above is supported. Note that ++ is still strict in the second parameter, which is why we also need to use it in combination with defer.

The challenge is that observable's operators need to return a Cancelable when the subscription happens. But this creates a memory leak. What we are doing here instead is to initialize a single MultiAssignmentCancelable in such a chain and to keep injecting it in subsequent subscriptions.

@alexandru alexandru changed the title from WIP: Make Observable concatenation (++) stack / memory safe to WIP: Make Observable concatenation (++) stack safe Aug 1, 2017

@codecov

This comment has been minimized.

codecov bot commented Aug 1, 2017

Codecov Report

Merging #391 into master will increase coverage by 0.03%.
The diff coverage is 64.7%.

@@            Coverage Diff            @@
##           master    #391      +/-   ##
=========================================
+ Coverage   87.57%   87.6%   +0.03%     
=========================================
  Files         293     294       +1     
  Lines        8104    8119      +15     
  Branches     1155    1174      +19     
=========================================
+ Hits         7097    7113      +16     
+ Misses       1007    1006       -1

@alexandru alexandru changed the title from WIP: Make Observable concatenation (++) stack safe to Make Observable concatenation (++) stack safe Aug 1, 2017

@alexandru alexandru merged commit 8089a88 into monix:master Aug 1, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alexandru alexandru added this to the 3.0.0 milestone Jan 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment