on master help sign in
Add some asserts, update docs
commit3a4e9163ea86642f4ce1f22e9cf3534b0c8db7c3 authorHarendra Kumar <harendra@composewell.com> dateWed 6 Oct 2021, 18:19 (5 years ago) parentc48ef04
2 files changed+79−51
src/Streamly/Internal/Data/Array/Foreign/Mut/Type.hs+57−42 src/Streamly/Internal/Data/Array/Foreign/Type.hs+22−9
src/Streamly/Internal/Data/Array/Foreign/Mut/Type.hs+57−42
@@ -145,6 +145,9 @@ plusForeignPtr :: ForeignPtr a -> Int -> ForeignPtr b
145145plusForeignPtr (ForeignPtr addr c) (I# d) = ForeignPtr (plusAddr# addr d) c
146146#endif
147147
148+-- $setup
149+-- >>> import qualified Streamly.Internal.Data.Stream.StreamD as StreamD
150+
148151-------------------------------------------------------------------------------
149152-- Array Data Type
150153-------------------------------------------------------------------------------
@@ -158,6 +161,8 @@ foreign import ccall unsafe "string.h memchr" c_memchr
158161foreign import ccall unsafe "string.h memcmp" c_memcmp
159162 :: Ptr Word8 -> Ptr Word8 -> CSize -> IO CInt
160163
164+-- | Given a 'Storable' type (unused first arg) and a number of bytes, return
165+-- how many elements of that type are required to hold those bytes.
161166{-# INLINE bytesToElemCount #-}
162167bytesToElemCount :: Storable a => a -> Int -> Int
163168bytesToElemCount x n =
@@ -176,6 +181,7 @@ memcmp p1 p2 len = do
176181 r <- c_memcmp p1 p2 (fromIntegral len)
177182 return $ r == 0
178183
184+-- | Same as unsafeWithForeignPtr but lifted to any MonadIO monad.
179185{-# INLINE unsafeWithForeignPtrM #-}
180186unsafeWithForeignPtrM :: MonadIO m => ForeignPtr a -> (Ptr a -> m b) -> m b
181187unsafeWithForeignPtrM fo f = do
@@ -193,8 +199,25 @@ unsafeWithForeignPtrM fo f = do
193199-- be automatically provided to a function that pattern matches on the Array
194200-- type. However, it has huge performance cost, so we do not use it.
195201-- Investigate a GHC improvement possiblity.
196−--
202+
197203-- XXX Rename the fields to better names.
204+
205+-- | An unboxed, pinned mutable array. An array is created with a given length
206+-- and capacity. Length is the number of valid elements in the array. Capacity
207+-- is the maximum number of elements that the array can be expanded to without
208+-- having to reallocate the memory.
209+--
210+-- The elements in the array can be mutated in-place without changing the
211+-- reference (constructor). However, the length of the array cannot be mutated
212+-- in-place. A new array reference is generated when the length changes. When
213+-- the length is increased (upto the maximum reserved capacity of the array),
214+-- the array is not reallocated and the new reference uses the same underlying
215+-- memory as the old one.
216+--
217+-- Several routines in this module allow the programmer to control the capacity
218+-- of the array. The programmer can control the trade-off between memory usage
219+-- and performance impact due to reallocations when growing or shrinking the
220+-- array.
198221--
199222data Array a =
200223#ifdef DEVBUILD
@@ -218,7 +241,7 @@ mutableArray = Array
218241-- Construction
219242-------------------------------------------------------------------------------
220243
221−-- | allocate a new array using the provided allocator function.
244+-- | allocate a new empty array using the provided allocator function.
222245{-# INLINE newArrayAlignedAllocWith #-}
223246newArrayAlignedAllocWith :: forall m a. (MonadIO m, Storable a)
224247 => (Int -> Int -> m (ForeignPtr a)) -> Int -> Int -> m (Array a)
@@ -232,9 +255,9 @@ newArrayAlignedAllocWith alloc alignSize count = do
232255 , aBound = p `plusPtr` size
233256 }
234257
235−-- | Allocate a new array aligned to the specified alignmend and using
258+-- | Allocate a new empty array using the specified alignment and using
236259-- unmanaged pinned memory. The memory will not be automatically freed by GHC.
237−-- This could be useful in allocate once global data structures. Use carefully
260+-- This could be useful in allocate-once global data structures. Use carefully
238261-- as incorrect use can lead to memory leak.
239262{-# INLINE newArrayAlignedUnmanaged #-}
240263newArrayAlignedUnmanaged :: (MonadIO m, Storable a) => Int -> Int -> m (Array a)
@@ -257,11 +280,9 @@ newArrayAligned = newArrayAlignedAllocWith mallocForeignPtrAlignedBytes
257280
258281-- XXX can unaligned allocation be more efficient when alignment is not needed?
259282--
260−-- | Allocate an array that can hold 'count' items. The memory of the array is
261−-- uninitialized.
283+-- | Allocate an empty array that can hold 'count' items. The memory of the
284+-- array is uninitialized.
262285--
263−-- Note that this is internal routine, the reference to this array cannot be
264−-- given out until the array has been written to and frozen.
265286{-# INLINE newArray #-}
266287newArray :: forall m a. (MonadIO m, Storable a) => Int -> m (Array a)
267288newArray = newArrayAligned (alignment (undefined :: a))
@@ -377,7 +398,7 @@ shrinkToFit arr@Array{..} = do
377398--
378399-------------------------------------------------------------------------------
379400
380−-- | Return element at the specified index without checking the bounds.
401+-- | Return the element at the specified index without checking the bounds.
381402--
382403-- Unsafe because it does not check the bounds of the array.
383404{-# INLINE_NORMAL unsafeIndex #-}
@@ -404,9 +425,16 @@ byteLength Array{..} =
404425 len = aEnd `minusPtr` p
405426 in assert (len >= 0) len
406427
428+-- Note: try to avoid the use of length in performance sensitive internal
429+-- routines as it involves a costly 'div' operation. Instead use the end ptr
430+-- int he array to check the bounds etc.
431+--
407432-- | /O(1)/ Get the length of the array i.e. the number of elements in the
408433-- array.
409434--
435+-- Note that 'byteLength' is less expensive than this operation, as 'length'
436+-- involves a costly division operation.
437+--
410438-- @since 0.7.0
411439{-# INLINE length #-}
412440length :: forall a. Storable a => Array a -> Int
@@ -455,8 +483,9 @@ arraysOf n (D.Stream step state) =
455483 step' _ (GroupStart st) = do
456484 when (n <= 0) $
457485 -- XXX we can pass the module string from the higher level API
458− error $ "Streamly.Internal.Data.Array.Foreign.Mut.Type.fromStreamDArraysOf: the size of "
459− ++ "arrays [" ++ show n ++ "] must be a natural number"
486+ error $ "Streamly.Internal.Data.Array.Foreign.Mut.Type.arraysOf: "
487+ ++ "the size of arrays [" ++ show n
488+ ++ "] must be a natural number"
460489 Array start end bound <- liftIO $ newArray n
461490 return $ D.Skip (GroupBuffer st start end bound)
462491
@@ -516,7 +545,7 @@ flattenArrays (D.Stream step state) = D.Stream step' (OuterLoop state)
516545 D.Skip s -> D.Skip (OuterLoop s)
517546 D.Stop -> D.Stop
518547
519− step' _ (InnerLoop st _ p end) | p == end =
548+ step' _ (InnerLoop st _ p end) | assert (p <= end) (p == end) =
520549 return $ D.Skip $ OuterLoop st
521550
522551 step' _ (InnerLoop st startf p end) = do
@@ -581,9 +610,10 @@ producer = Producer step inject extract
581610 return $ ReadUState (ForeignPtr end contents) (Ptr start)
582611
583612 {-# INLINE_LATE step #-}
584− step (ReadUState fp@(ForeignPtr end _) p) | p == Ptr end =
585− let x = unsafeInlineIO $ touchForeignPtr fp
586− in x `seq` return D.Stop
613+ step (ReadUState fp@(ForeignPtr end _) p)
614+ | assert (p <= Ptr end) (p == Ptr end) =
615+ let x = unsafeInlineIO $ touchForeignPtr fp
616+ in x `seq` return D.Stop
587617 step (ReadUState fp p) = do
588618 -- unsafeInlineIO allows us to run this in Identity monad for pure
589619 -- toList/foldr case which makes them much faster due to not
@@ -643,7 +673,7 @@ toListFB :: forall a b. Storable a => (a -> b -> b) -> b -> Array a -> b
643673toListFB c n Array{..} = go (unsafeForeignPtrToPtr aStart)
644674 where
645675
646− go p | p == aEnd = n
676+ go p | assert (p <= aEnd) (p == aEnd) = n
647677 go p =
648678 -- unsafeInlineIO allows us to run this in Identity monad for pure
649679 -- toList/foldr case which makes them much faster due to not
@@ -678,7 +708,7 @@ toStreamD Array{..} =
678708 where
679709
680710 {-# INLINE_LATE step #-}
681− step _ p | p == aEnd = return D.Stop
711+ step _ p | assert (p <= aEnd) (p == aEnd) = return D.Stop
682712 step _ p = do
683713 -- unsafeInlineIO allows us to run this in Identity monad for pure
684714 -- toList/foldr case which makes them much faster due to not
@@ -700,7 +730,7 @@ toStreamK Array{..} =
700730
701731 where
702732
703− go p | p == aEnd = K.nil
733+ go p | assert (p <= aEnd) (p == aEnd) = K.nil
704734 | otherwise =
705735 -- See Note in toStreamD.
706736 let !x = unsafeInlineIO $ do
@@ -852,7 +882,8 @@ writeNUnsafe = writeNUnsafeWith newArray
852882--
853883-- @writeChunks = Fold.many Fold.toStream (Array.writeN n)@
854884--
855−-- See 'bufferChunks'.
885+-- Breaking an array into an array stream can be useful to consume a large
886+-- array sequentially such that memory of the array is released incrementatlly.
856887--
857888-- /Unimplemented/
858889--
@@ -939,11 +970,12 @@ writeAligned alignSize =
939970
940971-- | Use the 'writeN' fold instead.
941972--
942−-- @fromStreamDN n = D.fold (writeN n)@
973+-- >>> fromStreamDN n = StreamD.fold (writeN n)
943974--
944975{-# INLINE_NORMAL fromStreamDN #-}
945976fromStreamDN :: forall m a. (MonadIO m, Storable a)
946977 => Int -> D.Stream m a -> m (Array a)
978+-- fromStreamDN n = D.fold (writeN n)
947979fromStreamDN limit str = do
948980 arr <- liftIO $ newArray limit
949981 end <- D.foldlM' fwrite (return $ aEnd arr) $ D.take limit str
@@ -1054,6 +1086,11 @@ spliceWithDoubling dst@(Array start end bound) src = do
10541086 let oldStart = unsafeForeignPtrToPtr start
10551087 oldSize = end `minusPtr` oldStart
10561088 newSize = max (oldSize * 2) (oldSize + srcLen)
1089+ when (newSize < oldSize + srcLen)
1090+ $ error
1091+ $ "spliceWithDoubling: newSize is less than the total size "
1092+ ++ "of arrays being appended. Please check the "
1093+ ++ "newSize function passed."
10571094 liftIO $ realloc newSize dst
10581095 else return dst
10591096 spliceWith dst1 src
@@ -1153,25 +1190,3 @@ instance (Storable a, Eq a) => Eq (Array a) where
11531190instance NFData (Array a) where
11541191 {-# INLINE rnf #-}
11551192 rnf Array {} = ()
1156−
1157−#ifdef DEVBUILD
1158−-- Definitions using the Storable constraint from the Array type. These are to
1159−-- make the Foldable instance possible though it is much slower (7x slower).
1160−--
1161−{-# INLINE_NORMAL toStreamD_ #-}
1162−toStreamD_ :: forall m a. MonadIO m => Int -> Array a -> D.Stream m a
1163−toStreamD_ size Array{..} =
1164− let p = unsafeForeignPtrToPtr aStart
1165− in D.Stream step p
1166−
1167− where
1168−
1169− {-# INLINE_LATE step #-}
1170− step _ p | p == aEnd = return D.Stop
1171− step _ p = do
1172− x <- liftIO $ do
1173− r <- peek p
1174− touchForeignPtr aStart
1175− return r
1176− return $ D.Yield x (p `plusPtr` size)
1177−#endif
src/Streamly/Internal/Data/Array/Foreign/Type.hs+22−9
@@ -153,10 +153,19 @@ foreign import ccall unsafe "string.h strlen" c_strlen
153153-- Freezing and Thawing
154154-------------------------------------------------------------------------------
155155
156−-- | Returns an immutable array using the same underlying pointers of the
157−-- mutable array. If the underlying array is mutated, the immutable promise is
158−-- lost. Please make sure that the mutable array is never used after freezing it
159−-- using /unsafeFreeze/.
156+-- XXX For debugging we can track slices/references through a weak IORef. Then
157+-- trigger a GC after freeze/thaw and assert that there are no references
158+-- remaining.
159+
160+-- | Makes an immutable array using the underlying memory of the mutable
161+-- array.
162+--
163+-- Please make sure that there are no other references to the mutable array
164+-- lying around, so that it is never used after freezing it using
165+-- /unsafeFreeze/. If the underlying array is mutated, the immutable promise
166+-- is lost.
167+--
168+-- /Pre-release/
160169{-# INLINE unsafeFreeze #-}
161170unsafeFreeze :: MA.Array a -> Array a
162171unsafeFreeze (MA.Array as ae _) = Array as ae
@@ -169,10 +178,14 @@ unsafeFreezeWithShrink arr = unsafePerformIO $ do
169178 MA.Array as ae _ <- MA.shrinkToFit arr
170179 return $ Array as ae
171180
172−-- | Returns a mutable array using the same underlying pointers of the immutable
173−-- array. If the resulting array is mutated, the older immutable array is
174−-- mutated as well. Please make sure that the immutable array is never used
175−-- after thawing it using /unsafeThaw/.
181+-- | Makes a mutable array using the underlying memory of the immutable array.
182+--
183+-- Please make sure that there are no other references to the immutable array
184+-- lying around, so that it is never used after thawing it using /unsafeThaw/.
185+-- If the resulting array is mutated, any references to the older immutable
186+-- array are mutated as well.
187+--
188+-- /Pre-release/
176189{-# INLINE unsafeThaw #-}
177190unsafeThaw :: Array a -> MA.Array a
178191unsafeThaw (Array as ae) = MA.Array as ae ae
@@ -416,7 +429,7 @@ byteLength = MA.byteLength . unsafeThaw
416429-- @since 0.8.0
417430{-# INLINE length #-}
418431length :: forall a. Storable a => Array a -> Int
419−length arr = MA.length (unsafeThaw arr)
432+length arr = MA.length (unsafeThaw arr)
420433
421434-- | Unfold an array into a stream in reverse order.
422435--