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

stack-safe IO #988

Closed
wants to merge 1 commit into from
Closed

stack-safe IO #988

wants to merge 1 commit into from

Conversation

giogonzo
Copy link
Collaborator

A porting of purescript/purescript-effect#12 by @safareli

related to #907

Pros

  • makes operations on IO stack safe (currently I get a "Maximum call stack size exceeded" error with e.g. array.sequence(io)(range(0, 10000).map(io.of))())
  • non-breaking / doesn't change the underlying representation of an IO<A> (a thunk () => A)
  • slightly better performance on a big number of operations on IO

Cons

  • private / obscure implementation details, makes the code harder to read
  • slightly worse performance on small number of operations on IO

Notes

  • I didn't spend time making the implementation better in terms of type-safety / style (not so relevant since private anyway), but it can definitely be done (e.g. using a proper discriminated union for the operation type, etc.)
  • Probably we can do something similar also for Task (see e.g. [Task] Maximum call stack size exceeded #983)?

Benchmarks

"big"
import * as Benchmark from 'benchmark'
import * as IO from '../src/IO'
import * as oldIO from '../src/old-IO'
import { array, range } from '../src/Array'
const suite = new Benchmark.Suite()

// old IO x 30.75 ops/sec ±0.81% (50 runs sampled)
// IO x 32.49 ops/sec ±0.61% (53 runs sampled)
// Fastest is IO

const arr = range(1, 6000)
suite
  .add('old IO', function() {
    array.sequence(oldIO.io)(arr.map(oldIO.of))()
  })
  .add('IO', function() {
    array.sequence(IO.io)(arr.map(IO.of))()
  })
  .on('cycle', function(event: any) {
    // tslint:disable-next-line: no-console
    console.log(String(event.target))
  })
  .on('complete', function(this: any) {
    // tslint:disable-next-line: no-console
    console.log('Fastest is ' + this.filter('fastest').map('name'))
  })
  .run({ async: true })
"small"
import * as Benchmark from 'benchmark'
import * as IO from '../src/IO'
import * as oldIO from '../src/old-IO'
import { array, range } from '../src/Array'
const suite = new Benchmark.Suite()

// old IO x 884,610 ops/sec ±0.82% (89 runs sampled)
// IO x 712,194 ops/sec ±0.66% (89 runs sampled)
// Fastest is old IO

const arr = range(1, 10)
suite
  .add('old IO', function() {
    array.sequence(oldIO.io)(arr.map(oldIO.of))()
  })
  .add('IO', function() {
    array.sequence(IO.io)(arr.map(IO.of))()
  })
  .on('cycle', function(event: any) {
    // tslint:disable-next-line: no-console
    console.log(String(event.target))
  })
  .on('complete', function(this: any) {
    // tslint:disable-next-line: no-console
    console.log('Fastest is ' + this.filter('fastest').map('name'))
  })
  .run({ async: true })

Opinions?

@safareli
Copy link

👍 Great to see this!

BTW using trivial implementation of traversable instance for array with other lazyish structures like Reader, State etc, will break with overflow when used on large array. it's possible to fix that for all such types like this sanctuary-js/sanctuary-type-classes#19

@giogonzo
Copy link
Collaborator Author

Interesting, that seems to be very similar to the implementation in https:/purescript/purescript-foldable-traversable/blob/master/src/Data/Traversable.js

array.sequence(S.state)(range(0, N).map(S.state.of))(undefined)

Is indeed stack overflowing for me with N greater than 5000

@giogonzo giogonzo closed this Jan 11, 2020
@safareli
Copy link

@giogonzo did you decide to fix Traversable instance instead?

@giogonzo
Copy link
Collaborator Author

@safareli no, we didn't do nothing (that I'm aware of) in the end.

Even if stack-safety-related issues come up from time to time, it doesn't seem to be a high priority for fp-ts at the moment (and there are workarounds, i.e. batching operations)

Also, the community is expanding in terms of higher-level libraries, and e.g. https:/Matechs-Garage/matechs-effect 's Effect is built on stack safe runtime (cc @mikearnaldi)

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

Successfully merging this pull request may close these issues.

2 participants