Implement Serialize instance for ByteString #12
Conversation
benchmark/Main.hs
Outdated
| import Data.ByteString as StrictByteString hiding (count) | ||
| import Data.ByteString.Lazy as LazyByteString hiding (count) |
| @@ -171,18 +173,27 @@ main = do | |||
| !lazyText <- do | |||
| testSList <- Stream.replicateM 20 (genStrictText 50) & Stream.toList | |||
| pure $ force $ TextL.fromChunks testSList | |||
| !strictByteString <- genStrictByteString 1000 | |||
| !lazyByteString <- genLazyByteString 10 100 -- 100 Strict bytestrings each of len 10 | |||
There was a problem hiding this comment.
You can keep the distribution the same as the lazy text so we can compare them fairly.
benchmark/Main.hs
Outdated
| unless (StrictByteString.length strictByteString == 1000) | ||
| (error "TextS.length strictText == 1000") | ||
| unless (LazyByteString.length lazyByteString == 1000) | ||
| (error "TextS.length strictText == 1000") |
There was a problem hiding this comment.
Change the error message accordingly.
benchmark/Main.hs
Outdated
| import Streamly.Internal.Data.Unbox (newBytes, MutableByteArray) | ||
| import Streamly.Internal.Data.Serialize hiding (encode) | ||
|
|
||
| import qualified Streamly.Data.Stream as Stream | ||
| import qualified Data.Text as TextS | ||
| import qualified Data.Text.Lazy as TextL | ||
|
|
||
| import Data.ByteString as StrictByteString hiding (count) | ||
| import Data.ByteString.Lazy as LazyByteString hiding (count) | ||
| import Data.Word (Word8) |
There was a problem hiding this comment.
Move this to the top. Keep this along with other explicit imports.
benchmark/Main.hs
Outdated
|
|
||
| -- Benchmarks | ||
| defaultMain | ||
| [ bencher "[Int]" intList 100 | ||
| , bencher "Strict.Text" strictText 100 | ||
| , bencher "Lazy.Text" lazyText 100 | ||
| , bencher "Strict.ByteString" strictByteString 100 | ||
| , bencher "Strict.LazyByteString" lazyByteString 100 |
There was a problem hiding this comment.
Strict.LazyByteString -> Lazy.ByteString
| & Stream.toList | ||
| & fmap (force . StrictByteString.pack) | ||
|
|
||
| genLazyByteString n m = do |
There was a problem hiding this comment.
Even the lazy text generator can be separated out like this.
| import Streamly.Internal.Data.Serialize (Serialize(..)) | ||
| import Data.ByteString.Internal as Strict | ||
| import Data.ByteString.Lazy as Lazy | ||
| import qualified Streamly.Internal.Data.Unbox as Unbox | ||
| import GHC.Exts | ||
| import Foreign.ForeignPtr (withForeignPtr) | ||
| import Foreign.Ptr (plusPtr) | ||
| import GHC.Base (IO(..)) | ||
| import Control.Monad (foldM_) |
There was a problem hiding this comment.
Keep the explicit imports first, qualified imports next, and the rest after that.
Import qualified where possible.
|
|
||
| -------------------------------------------------------------------------------- | ||
| -- Strict ByteString | ||
| -------------------------------------------------------------------------------- |
| {-# INLINE deserialize #-} | ||
| deserialize off arr end = do | ||
| (off1, lenBytes) <- deserialize off arr end :: IO (Int, Int) | ||
| fp <- Strict.mallocByteString lenBytes |
There was a problem hiding this comment.
Any reason to use Strict.mallocByteString over Strict.create?
Strict.create seems to have some more functionality w.r.t safety that incurs no overhead.
| withForeignPtr fp $ \(Ptr addr#) -> IO $ \s# -> (# copyMutableByteArrayToAddr# | ||
| arrS# srcStartBytes# addr# lenBytes# s# | ||
| , () #) |
There was a problem hiding this comment.
Indent this properly. 80 characters is the line limit.
| withForeignPtr fp $ \srcPtr -> let !(Ptr addr#) = srcPtr `plusPtr` srcOffset | ||
| in IO $ \s# -> (# copyAddrToByteArray# | ||
| addr# arrD# dstStartBytes# lenBytes# s# | ||
| , () #) |
There was a problem hiding this comment.
Indent this properly, 80 characters is the line limit.
| withForeignPtr fp $ \srcPtr -> let !(Ptr addr#) = srcPtr `plusPtr` srcOffset | ||
| in IO $ \s# -> (# copyAddrToByteArray# | ||
| addr# arrD# dstStartBytes# lenBytes# s# | ||
| , () #) |
There was a problem hiding this comment.
| withForeignPtr fp $ \srcPtr -> let !(Ptr addr#) = srcPtr `plusPtr` srcOffset | |
| in IO $ \s# -> (# copyAddrToByteArray# | |
| addr# arrD# dstStartBytes# lenBytes# s# | |
| , () #) | |
| withForeignPtr fp $ \srcPtr -> | |
| let !(Ptr addr#) = srcPtr `plusPtr` srcOffset | |
| in IO $ \s# -> | |
| (# copyAddrToByteArray# addr# arrD# dstStartBytes# lenBytes# s# | |
| , () #) |
There was a problem hiding this comment.
The suggestion is still not perfect but this will do.
|
|
||
| {-# INLINE serialize #-} | ||
| serialize off arr (Strict.PS fp srcOffset lenBytes) = do | ||
| off1 <- serialize off arr lenBytes |
There was a problem hiding this comment.
Use machine-independent types while serializing. Use Int64 instead of Int
|
|
||
| {-# INLINE deserialize #-} | ||
| deserialize off arr end = do | ||
| (off1, lenBytes) <- deserialize off arr end :: IO (Int, Int) |
There was a problem hiding this comment.
Use Int64 over Int. Use machine-independent types.
| -------------------------------------------------------------------------------- | ||
| instance Serialize Strict.ByteString where | ||
| {-# INLINE size #-} | ||
| size i (Strict.PS _ _ l) = i + l + size 0 l |
There was a problem hiding this comment.
You can use Unbox.sizeOf (Proxy :: Proxy Int64) or simply just use 8 instead of size 0 l.
i + l + 8
Or,
i + l + Unbox.sizeOf (Proxy :: Proxy Int64)
| -------------------------------------------------------------------------------- | ||
| -- Lazy ByteString | ||
| -------------------------------------------------------------------------------- | ||
| instance Serialize Lazy.ByteString where |
There was a problem hiding this comment.
You can use the template haskell helper for deriving serialization for lazy bytestring.
$(Serialize.deriveSerialize ''Lazy.ByteString)
should work.
Although I'm not sure if it will be portable across bytestring upgrades.
Changing the name and order of constructors will change the deriving strategy.
We should have more config options in the template haskell helpers with which we can give an order to the constructors manually.
There was a problem hiding this comment.
| @@ -0,0 +1,84 @@ | |||
| {-# LANGUAGE DataKinds #-} | |||
| {-# LANGUAGE UnboxedTuples #-} | |||
| {-# LANGUAGE UndecidableInstances #-} | |||
There was a problem hiding this comment.
Document why do you need this extension.
|
Add version constraints to bytestring in the cabal file. |
No description provided.