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

Optimize `materialize`, introduce `transformWith`, refactor run-loop #353

Merged
merged 8 commits into from Apr 30, 2017

Conversation

Projects
None yet
1 participant
@alexandru
Copy link
Member

commented Apr 30, 2017

Trying out a cats-effect integration, I stumbled upon this sample in one of the laws that Task must pass:

    val loop = (0 until 10000).foldLeft(Task.eval(0)) { (acc, _) =>
      acc.materialize.flatMap {
        case Success(x) =>
          Task.now(x + 1)
        case Failure(ex) =>
          Task.raiseError(ex)
      }
    }

So what this does is to eagerly apply materialize and flatMap in a foldLeft. This ends up building a big data-structure in memory and unfortunately the Task freezes, as with the current encoding of materialize it cannot handle this load.

This prompted the following changes:

  1. introduce transform and transformWith operations that are treated as first-class citizens in the Task implementation, being handled by the internal FlatMap state
  2. the implementation builds a FlatMap state that uses a new Transformation[-A, +B] type, which is a plain function + an error handler
  3. the run-loop, upon encountering an error, unwinds its internal call stack until hitting an available Transformation, which is then used to handle the error
  4. did the same for Coeval, introduced transform and transformWith
  5. changed the Coeval.FlatMap state to behave like that of Task

Also we have the following refactorings:

  1. moved Task's run-loop implementation in monix.eval.internals.TaskRunLoop
  2. moved Coeval's run-loop implementation in monix.eval.internals.CoevalRunLoop
@codecov

This comment has been minimized.

Copy link

commented Apr 30, 2017

Codecov Report

Merging #353 into master will decrease coverage by 0.03%.
The diff coverage is 93.26%.

@@            Coverage Diff             @@
##           master     #353      +/-   ##
==========================================
- Coverage   86.79%   86.76%   -0.04%     
==========================================
  Files         293      295       +2     
  Lines        8362     8357       -5     
  Branches     1683     1663      -20     
==========================================
- Hits         7258     7251       -7     
- Misses       1104     1106       +2
@codecov

This comment has been minimized.

Copy link

commented Apr 30, 2017

Codecov Report

Merging #353 into master will increase coverage by 0.17%.
The diff coverage is 96.84%.

@@            Coverage Diff             @@
##           master     #353      +/-   ##
==========================================
+ Coverage   86.79%   86.96%   +0.17%     
==========================================
  Files         293      296       +3     
  Lines        8362     8356       -6     
  Branches     1683     1675       -8     
==========================================
+ Hits         7258     7267       +9     
+ Misses       1104     1089      -15

@alexandru alexandru changed the title WIP: Optimize `materialize`, introduce `transformWith`, refactor run-loop Optimize `materialize`, introduce `transformWith`, refactor run-loop Apr 30, 2017

@alexandru alexandru merged commit c936d43 into master Apr 30, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@alexandru alexandru deleted the run-optim branch May 4, 2017

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.