Lazier unlines

[noughtmare]

Fixes #476

Benchmark results:

before:

  lazy-unlines: OK (0.30s)
    547  μs ±  34 μs, 3.2 MB allocated,  23 KB copied,  38 MB peak memory

after:

  lazy-unlines: OK (0.20s)
    354  μs ±  24 μs, 2.0 MB allocated,  23 KB copied,  38 MB peak memory

[sjakobi]

It would be good to have a test for the laziness property or at least a comment that points out this requirement for the implementation.

[noughtmare]

Should I also change the strict version? It won't benefit from the strictness changes and a similar benchmark doesn't show very significant changes, but it does make sense to implement both functions similarly and this new implementation is simpler.

[noughtmare]

@sjakobi I've added a test that checks the laziness of unlines. It fails with the old implementation.

[clyring]

It's not directly related to the motivation for this patch, but does performance of lazy unlines further improve if concat is also rewritten as a foldr, so that the List.concatMap can potentially fuse with it? And are there any drawbacks to doing so?

[sjakobi]

@noughtmare What's the performance impact on the strict version?

@clyring Good question. I think it would be best to address it separately from this PR though, to keep the size of this one more manageable and to avoid a long-standing branch that might acquire merge conflicts. Feel free to record the idea on the issue tracker.

[noughtmare]

What's the performance impact on the strict version?

Before:

strict: OK (2.44s)
  292  μs ± 7.3 μs, 876 KB allocated, 5.7 KB copied,  40 MB peak memory

After:

strict: OK (0.59s)
  279  μs ± 6.8 μs, 739 KB allocated, 5.3 KB copied,  38 MB peak memory

[sjakobi]

Thanks!

[Bodigrim]

This is better than before, because it avoids copying the full output twice (with and without final nl).
Ideally unlines should be implemented in a style similar to intercalate, avoiding concat (which is quite ineffective):

bytestring/Data/ByteString.hs

Lines 1209 to 1227 in 5efd6b5

	intercalate :: ByteString -> [ByteString] -> ByteString
	intercalate _ [] = mempty
	intercalate _ [x] = x -- This branch exists for laziness, not speed
	intercalate (BS fSepPtr sepLen) (BS fhPtr hLen : t) =
	unsafeCreate totalLen $ \dstPtr0 ->
	unsafeWithForeignPtr fSepPtr $ \sepPtr -> do
	unsafeWithForeignPtr fhPtr $ \hPtr ->
	memcpy dstPtr0 hPtr hLen
	let go _ [] = pure ()
	go dstPtr (BS fChunkPtr chunkLen : chunks) = do
	memcpy dstPtr sepPtr sepLen
	let destPtr' = dstPtr `plusPtr` sepLen
	unsafeWithForeignPtr fChunkPtr $ \chunkPtr ->
	memcpy destPtr' chunkPtr chunkLen
	go (destPtr' `plusPtr` chunkLen) chunks
	go (dstPtr0 `plusPtr` hLen) t
	where
	totalLen = List.foldl' (\acc chunk -> acc +! sepLen +! length chunk) hLen t
	(+!) = checkedAdd "intercalate"

[Bodigrim]

Thanks @noughtmare!