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

Foldable instance for Vector uses slow default implementations for many methods #112

Closed
mgsloan opened this issue Mar 23, 2016 · 6 comments

Comments

@mgsloan
Copy link

mgsloan commented Mar 23, 2016

The current definition is

instance Foldable.Foldable Vector where
  {-# INLINE foldr #-}
  foldr = foldr

  {-# INLINE foldl #-}
  foldl = foldl

  {-# INLINE foldr1 #-}
  foldr1 = foldr1

  {-# INLINE foldl1 #-}
  foldl1 = foldl1

The problem with this is that there are many other Foldable operations that have Vector versions which are much faster than the default implementations. Sometimes Vector even offers an asymptotically superior version, such as length, which has an O(n) default implementation, whereas Vector has a O(1) operation for this.

I would open a PR, but was dissuaded by the pile of existing ones.

@winterland1989
Copy link

Please go ahead, this is clearly an optimazation which won't break any API, who is the maintainer now? Please make a judgement ; )

@Shimuuar
Copy link
Contributor

who is the maintainer now?

I'd love to know answer to this question.

@dolio
Copy link
Contributor

dolio commented Jul 19, 2016

The core libraries committee is the official maintainer, and they delegate mostly to me.

If someone wants to implement this, it sounds like a good idea to me.

@Shimuuar
Copy link
Contributor

Well judging by number of open pull requests and commit history there isn't enough manpower to maintain vector properly.

@cartazio
Copy link
Contributor

could someone roll a wee benchmark to validate switching Data.Vector and suchlike to this?

Relatedly: I presume we can only provide a Foldable instance for boxed vectors right?

@dolio
Copy link
Contributor

dolio commented Jul 24, 2016

I implemented this. And yes, it's only for boxed vectors (Foldable could be implemented for unboxed vectors, but only by packing the dictionary in the type).

@dolio dolio closed this as completed Jul 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants