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 <* for sequences #314

Closed
treeowl opened this issue Aug 2, 2016 · 4 comments · Fixed by #713
Closed

Optimize <* for sequences #314

treeowl opened this issue Aug 2, 2016 · 4 comments · Fixed by #713
Assignees

Comments

@treeowl
Copy link
Contributor

treeowl commented Aug 2, 2016

We should be able to optimize <* for sequences to get more sharing within subtrees and go faster. Basically, we want to use <$ to make subtrees instead of <$>.

We want to maintain the incremental bounds. In particular, (xs <* ys) `index` k should produce a result in logarithmic time.

@treeowl
Copy link
Contributor Author

treeowl commented Aug 2, 2016

This looks trickier than I thought! I suspect doing this well will require rather more work than <* did.

@treeowl
Copy link
Contributor Author

treeowl commented Apr 11, 2019

@oisdk, I haven't been able to give this enough sustained attention to get it done. Do you think you can have a look? It has to be a sort of mash-up of things from aptyMiddle with things from applicativeTree. Instead of actually rigidifying the second tree, we must instead track the sizes the tree components should have (or something like that). The function we build going down (corresponding to map23) has to produce 2-3 trees with maximal sharing. I expect the function corresponding to aptyMiddle will have a type similar to

beforeMiddle
  :: c
  -> c
  -> (a -> c)
  -> FingerTree (Elem a)
  -> Int
  -> Int
  -> FingerTree (Node c)

but that could be wrong.

@oisdk
Copy link
Contributor

oisdk commented Apr 11, 2019

Yeah I can take a look!

@treeowl
Copy link
Contributor Author

treeowl commented Apr 11, 2019

I'd expect m <* n to take O(|m| log |n|) time and space. It currently takes a full O(|m||n|).

@oisdk oisdk self-assigned this Apr 11, 2019
@oisdk oisdk mentioned this issue May 14, 2019
@treeowl treeowl assigned treeowl and unassigned oisdk Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants