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

Implement Applicative based API #522

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Implement Applicative based API #522

wants to merge 4 commits into from

Conversation

Shimuuar
Copy link
Contributor

Implementation follows plan described in #477 with generateA :: Applicative f => Int -> (Int -> f a) -> f (v a) as primitive. It is not subject to stream fusion.

Writing generateA is not difficult. Problem is doing it efficiently. Current benchmarks are (suggestions for more are very welcome):

  1. Generate vector of random numbers using state monad
  2. Generate vector using IO action
  3. Compute sum of vector using lens
  4. Map vector using lens

First and naive implementation uses list as intermediate data structure. Sum benchmark performs well in this case. Using STA instead brings map benchmark on par with explicit loop and produces slight (5-10%) improvements in state and IO benchmark)

newtype STA v a = STA {  _runSTA :: forall s. Mutable v s a -> ST s (v a) }

Currently sum and map perform on par with explicit loop. State gives 7x slowdown and 8x allocations, IO benchmark 4x slowdown and 8x allocations. We obviously can add rewrite rules for IO/ST but maybe there're more general optimizations.


Fixes #477, #69, #132, #144

generateA is used as primitive and all other functions are expressed in it
terms. First version goes through intermediate list.

This is simplest implementation possible and would serve as baseline for
further optimizations
We establish implementation which goes through list as baseline and the we can
try to optimize it.

Note definition of foldlOf'. It's different from definition in lens<=5.3.3
but it's absolutely necessary to get good perfomance in folds
Does wonders for traversals using Identity
Copy link
Contributor

@konsumlamm konsumlamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about for/for_/traverse_ etc?

vector/src/Data/Vector/Generic.hs Outdated Show resolved Hide resolved
go !i | i >= n = pure $ STA unsafeFreeze
| otherwise = (\a (STA m) -> STA $ \mv -> M.unsafeWrite mv i a >> m mv)
<$> f i
<*> go (i + 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably better to use liftA2 here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. But I'm not sure that STA will survive maybe some New-like variant will perform better

Co-authored-by: konsumlamm <44230978+konsumlamm@users.noreply.github.com>
@Shimuuar
Copy link
Contributor Author

What about for/for_/traverse_ etc?

Good point! I forgot about them. But they're simpler since in this case one isn't in business of constructing vector so it simply reduces to fold.

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.

API using Applicative (traverse et.al.)
2 participants