Skip to content

add strict map functions for Array and SmallArray #138

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 19, 2018
Merged

add strict map functions for Array and SmallArray #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7cfd83
add strict map functions for Array and SmallArray
andrewthad Apr 19, 2018
08624c5
explain why indexArrayM is used when mapping over an array
andrewthad Apr 19, 2018
8925df1
Update Array.hs
treeowl Apr 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Data/Primitive/Array.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ module Data.Primitive.Array (
cloneArray, cloneMutableArray,
sizeofArray, sizeofMutableArray,
fromListN, fromList,
mapArray',
unsafeTraverseArray
) where

Expand Down Expand Up @@ -559,6 +560,21 @@ unsafeTraverseArray f = \ !ary ->
go 0 mary
{-# INLINE unsafeTraverseArray #-}

-- | Strict map over the elements of the array.
mapArray' :: (a -> b) -> Array a -> Array b
mapArray' f a =
createArray (sizeofArray a) (die "mapArray'" "impossible") $ \mb ->
let go i | i == sizeofArray a
= return ()
| otherwise
= do x <- indexArrayM a i
Copy link
Collaborator

@treeowl treeowl Apr 19, 2018

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

@andrewthad andrewthad Apr 19, 2018

Choose a reason for hiding this comment

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

Done.

-- We use indexArrayM here so that we will perform the
-- indexing eagerly even if f is lazy.
let !y = f x
writeArray mb i y >> go (i+1)
in go 0
{-# INLINE mapArray' #-}

arrayFromListN :: Int -> [a] -> Array a
arrayFromListN n l =
createArray n (die "fromListN" "uninitialized element") $ \sma ->
Expand Down
15 changes: 15 additions & 0 deletions Data/Primitive/SmallArray.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ module Data.Primitive.SmallArray
, sizeofSmallMutableArray
, smallArrayFromList
, smallArrayFromListN
, mapSmallArray'
, unsafeTraverseSmallArray
) where

Expand Down Expand Up @@ -436,6 +437,20 @@ unsafeTraverseSmallArray f (SmallArray ar) = SmallArray `liftM` unsafeTraverseAr
#endif
{-# INLINE unsafeTraverseSmallArray #-}

-- | Strict map over the elements of the array.
mapSmallArray' :: (a -> b) -> SmallArray a -> SmallArray b
#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

@treeowl treeowl Apr 19, 2018

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

@andrewthad andrewthad Apr 19, 2018

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

@treeowl treeowl Apr 19, 2018

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

@andrewthad andrewthad Apr 19, 2018

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.

x <- indexSmallArrayM sa i
let !y = f x
writeSmallArray smb i y *> go (i+1)
#else
mapSmallArray' f (SmallArray ar) = SmallArray (mapArray' f ar)
#endif
{-# INLINE mapSmallArray' #-}

#ifndef HAVE_SMALL_ARRAY
runSmallArray
:: (forall s. ST s (SmallMutableArray s a))
Expand Down
14 changes: 8 additions & 6 deletions test/main.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,31 +45,33 @@ main = do
, lawsToTest (QCC.ordLaws (Proxy :: Proxy (Array Int)))
, lawsToTest (QCC.monoidLaws (Proxy :: Proxy (Array Int)))
, lawsToTest (QCC.showReadLaws (Proxy :: Proxy (Array Int)))
#if MIN_VERSION_base(4,7,0)
, lawsToTest (QCC.isListLaws (Proxy :: Proxy (Array Int)))
#endif
#if MIN_VERSION_base(4,9,0) || MIN_VERSION_transformers(0,4,0)
, lawsToTest (QCC.functorLaws (Proxy1 :: Proxy1 Array))
, lawsToTest (QCC.applicativeLaws (Proxy1 :: Proxy1 Array))
, lawsToTest (QCC.monadLaws (Proxy1 :: Proxy1 Array))
, lawsToTest (QCC.foldableLaws (Proxy1 :: Proxy1 Array))
, lawsToTest (QCC.traversableLaws (Proxy1 :: Proxy1 Array))
#endif
#if MIN_VERSION_base(4,7,0)
, lawsToTest (QCC.isListLaws (Proxy :: Proxy (Array Int)))
, TQC.testProperty "mapArray'" (QCCL.mapProp int16 int32 mapArray')
#endif
]
, testGroup "SmallArray"
[ lawsToTest (QCC.eqLaws (Proxy :: Proxy (SmallArray Int)))
, lawsToTest (QCC.ordLaws (Proxy :: Proxy (SmallArray Int)))
, lawsToTest (QCC.monoidLaws (Proxy :: Proxy (SmallArray Int)))
, lawsToTest (QCC.showReadLaws (Proxy :: Proxy (Array Int)))
#if MIN_VERSION_base(4,7,0)
, lawsToTest (QCC.isListLaws (Proxy :: Proxy (SmallArray Int)))
#endif
#if MIN_VERSION_base(4,9,0) || MIN_VERSION_transformers(0,4,0)
, lawsToTest (QCC.functorLaws (Proxy1 :: Proxy1 SmallArray))
, lawsToTest (QCC.applicativeLaws (Proxy1 :: Proxy1 SmallArray))
, lawsToTest (QCC.monadLaws (Proxy1 :: Proxy1 SmallArray))
, lawsToTest (QCC.foldableLaws (Proxy1 :: Proxy1 SmallArray))
, lawsToTest (QCC.traversableLaws (Proxy1 :: Proxy1 SmallArray))
#endif
#if MIN_VERSION_base(4,7,0)
, lawsToTest (QCC.isListLaws (Proxy :: Proxy (SmallArray Int)))
, TQC.testProperty "mapSmallArray'" (QCCL.mapProp int16 int32 mapSmallArray')
#endif
]
, testGroup "ByteArray"
Expand Down