Skip to content

Conversation

@treeowl
Copy link
Collaborator

@treeowl treeowl commented Mar 18, 2018

  • Instead of traversing a list and then converting to an array,
    be more direct. GHC seems to be able to optimize this better,
    at least in common cases.

  • Add functions for traversing arrays in "affine" primitive monads: monad transformer stacks built from StateT, ReaderT, WriterT, RWST, ExceptT, Stream (Of a), etc., but not, for example, ListT (done right or otherwise) or LogicT.

Fixes #85.

@treeowl
Copy link
Collaborator Author

treeowl commented Mar 18, 2018

Do we still support pre-7.2 GHC, or can I kill that CPP?

@cartazio
Copy link
Contributor

cartazio commented Mar 18, 2018 via email

@RyanGlScott
Copy link
Member

Yep, we only support GHC 7.4 and later (see #83 (comment)), so go ahead and kill that CPP.

@treeowl treeowl force-pushed the traverse-better branch 2 times, most recently from e006ec4 to 6f3d4ab Compare March 18, 2018 16:40
@treeowl
Copy link
Collaborator Author

treeowl commented Mar 18, 2018

I amended the commit to avoid adding 7.2-related CPP, and added a commit to remove the rest of said CPP from Data.Primitive.Array.

@treeowl
Copy link
Collaborator Author

treeowl commented Mar 18, 2018

Hrm..... Turned out what I was doing to turn the letrec to a joinrec in the identity case was a bit strictness-risky in the general case. I'm not sure why the join point stuff is fragile here. But I decided to just do the simple thing for now. We can revisit it some other time.

@treeowl
Copy link
Collaborator Author

treeowl commented Mar 18, 2018

Actually, I think I see how to fix that. It's a bit ugly, and only available for recent GHC, but I think I'll do it anyway. One more fix incoming...

@treeowl
Copy link
Collaborator Author

treeowl commented Mar 18, 2018

(Ugly as in ugly source code. It's actually cleaner in a sense.

@treeowl
Copy link
Collaborator Author

treeowl commented Mar 18, 2018

Oh, actually, I can make it work for older versions too, with a perfectly safe unsafeCoerce#. Much less mess that way.

@treeowl
Copy link
Collaborator Author

treeowl commented Mar 18, 2018

Eh, I changed my mind. Source code gets too messy, and the benefits seem too limited. I think this is ready to merge as is.

@treeowl treeowl force-pushed the traverse-better branch 7 times, most recently from b6157c7 to 6e92476 Compare March 19, 2018 06:20
treeowl added 3 commits March 19, 2018 14:05
Instead of traversing a list and then converting to an array,
be more direct. GHC seems to be able to optimize this better,
at least in common cases.

Addresses haskell#85.
* Add `unsafeTraverseArray` and `unsafeTraverseSmallArray`
  functions.

* Add rewrite rules to use them for traversals in `ST s`
  and `IO`.

* Add rewrite rules for traversing in `Identity`.
Copy link
Member

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

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

This looks entirely plausible to me. Like foldl, it might be nice to write some QuickCheck tests for this, but I'd be fine if you want to merge this now.

@treeowl treeowl merged commit dc89b87 into haskell:master Mar 20, 2018
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.

3 participants