Skip to content

Conversation

@andrewthad
Copy link
Collaborator

No description provided.

let go i | i == sizeofArray a
= return ()
| otherwise
= do x <- indexArrayM a i
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be worth mentioning in a comment that we use indexArrayM here in case f is lazy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

#if HAVE_SMALL_ARRAY
mapSmallArray' f sa = createSmallArray (length sa) (die "mapSmallArray'" "impossible") $ \smb ->
fix ? 0 $ \go i ->
when (i < length sa) $ do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why < here and == for Array?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not really sure. I just copied the implementation of fmap for SmallArray to get this. I think the better question is why does everything in SmallArray use fix while everything in Array uses explicit recursion?

Copy link
Collaborator

Choose a reason for hiding this comment

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

History, I imagine. The fix seems to avoid the need for a local type signature in the go function. I have no idea how the generated code compares, but that would be worth checking.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm inclined to leave this as is for now. I'd like to open a separate PR adding benchmarks that check the difference between these two styles.

@treeowl
Copy link
Collaborator

treeowl commented Apr 19, 2018

I cut down and reworded the comment. There are two reasons to want to perform the lookup eagerly, and this is not the place to explain that. Aside from holding references to the array, we don't want to build a thunk to perform the lookup.

@treeowl treeowl merged commit 707209c into haskell:master Apr 19, 2018
@andrewthad
Copy link
Collaborator Author

Your wording is certainly more concise. Thanks!

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.

2 participants