Skip to content

Use an index instead of uncons to make array functions more efficient. #71

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

Closed
jutaro opened this issue Oct 4, 2016 · 7 comments
Closed

Comments

@jutaro
Copy link

jutaro commented Oct 4, 2016

The implementation of many array functions, like filterM, span, groupBy, ... uses uncons for implementation. It would be more efficient to use an index and has no known disadvantages (other then maybe some subjective aesthetic property).

If we can agree on this I can contribute code, but I don't want to work on this if it is not wanted.

@paf31
Copy link
Contributor

paf31 commented Oct 4, 2016

Let's list out the functions which have possible issues, since I think each could have a slightly different fix.

filterM - I'm not sure how you could make this more efficient for a general Monad.

span and groupBy could potentially make use of STArray. Something like discrimination is probably worth investigating for sorting and grouping too.

@hdgarrood
Copy link
Contributor

I think converting to a list before doing filterM and then converting back to an Array at the end would be better than the current implementation; right now it's at least O(n^2) I think.

hdgarrood added a commit that referenced this issue Nov 20, 2016
Change filterM to just be filterA in order to have this be a
non-breaking change.

This removes another use of uncons, refs #71
hdgarrood added a commit that referenced this issue Nov 20, 2016
Change filterM to just be filterA in order to have this be a
non-breaking change.

This removes another use of uncons, refs #71
@hdgarrood
Copy link
Contributor

Mostly a note to myself: now, it's just groupBy, nubBy, unzip, and foldM that use uncons (and perhaps shouldn't).

hdgarrood added a commit that referenced this issue Jan 19, 2017
Refs #71, improves performance of groupBy.
@hdgarrood
Copy link
Contributor

hdgarrood commented Feb 6, 2017

groupBy is now done, so I'll make a checklist for the remaining ones:

  • nubBy (now called nubByEq in the 0.12 branch)
  • unzip
  • foldM

hdgarrood added a commit that referenced this issue Jun 20, 2017
hdgarrood added a commit that referenced this issue Jun 20, 2017
@matthewleon
Copy link
Contributor

how about having foldM just be a re-export?

@hdgarrood
Copy link
Contributor

hdgarrood commented Jan 22, 2018

That sounds good to me, provided that the Foldable version has better performance than the current foldM — which is almost certainly true but probably still worth checking, at least for one or two monads.

matthewleon added a commit to matthewleon/purescript-arrays that referenced this issue Feb 10, 2018
sharno added a commit to sharno/purescript-arrays that referenced this issue Feb 11, 2019
garyb added a commit that referenced this issue Mar 31, 2019
make nubByEq more efficient, part of #71
@hdgarrood
Copy link
Contributor

nubByEq was the last problematic one, so I'm going to close this, but of course please continue to open issues if you find places where performance could be improved.

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

No branches or pull requests

4 participants