Skip to content

Commit

Permalink
Fix incorrect compression resulting from input vs output confusion.
Browse files Browse the repository at this point in the history
Also adds a test that fails without this fix.

`BS.drop writtenInt bs` made no sense, given that `bs` is the *input* ByteString;
neither did `writtenInt < BS.length bs`.
This is because `writtenInt` in the number of compressed Bytes output.

What I originally had in mind was to drop the number of consumed input Bytes.
But `LZ4F_compressUpdate()` does not return those, because it *always*
consumes the full input (and sizing the output buffer based on
`lz4fCompressBound()` ensures that it always fits into the output).

That means that the check whether it fits is not necessary at all.

This commit fixes the issue by removing the incorrect and unnecessary check.

But the whole concept of `loopSingleBs` is unnecessary, because we we never
need to loop over a single BS.
The next commit will remove this unnecessary logic.
  • Loading branch information
nh2 committed Sep 12, 2020
1 parent a282c60 commit 75ecc64
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
1 change: 0 additions & 1 deletion src/Codec/Compression/LZ4/Conduit.hsc
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,6 @@ compressWithOutBufferSize bufferSize =

if
| written == 0 -> return newRemainingCapacity
| writtenInt < BS.length bs -> loopSingleBs newRemainingCapacity (BS.drop writtenInt bs)
| otherwise -> return newRemainingCapacity

loop (outBufferSize - headerSize)
Expand Down
11 changes: 11 additions & 0 deletions test/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import qualified Data.ByteString as BS
import qualified Data.ByteString.Lazy as BSL
import qualified Data.ByteString.Lazy.Char8 as BSL8
import Data.Conduit
import Data.Conduit.Binary as CB
import qualified Data.Conduit.List as CL
import qualified Data.Conduit.Process as CP
import Data.List (intersperse)
Expand Down Expand Up @@ -57,6 +58,11 @@ main = do
actual <- runCompressToLZ4 (CL.sourceList strings)
actual `shouldBe` (BS.concat strings)

it "compresses 1MB ByteString" $ do
let bs = BS.replicate 100000 42
actual <- runCompressToLZ4 (CB.sourceLbs $ BSL.fromStrict bs)
actual `shouldBe` bs

describe "Decompression" $ do
it "decompresses simple string" $ do
let string = "hellohellohellohello"
Expand All @@ -73,6 +79,11 @@ main = do
actual <- runLZ4ToDecompress (CL.sourceList strings)
actual `shouldBe` (BS.concat strings)

it "decompresses 1MB ByteString" $ do
let bs = BS.replicate 100000 42
actual <- runLZ4ToDecompress (CB.sourceLbs $ BSL.fromStrict bs)
actual `shouldBe` bs

describe "Identity" $ do
modifyMaxSize (const 10000) $ it "compress and decompress arbitrary strings"$
QC.property $ \string -> QCM.monadicIO $ do
Expand Down

0 comments on commit 75ecc64

Please sign in to comment.