linear-builder icon indicating copy to clipboard operation
linear-builder copied to clipboard

Add an API for the ByteString builder

Open wismill opened this issue 1 year ago • 5 comments

I just realized that this package uses a ByteString builder to implement (|>%) and (%<|).

I did play a bit with them in #20, but felt uncomfortable with unsafePerformIO, etc. I then tried with Data.ByteString.Lazy.foldlChunks, which obviously would not lead to great perf, giving we need to allocate intermediate chunks and then copy them.

Should we provide an API for the ByteString builder? It would unlock quite a lot of features.

wismill avatar May 25 '24 10:05 wismill

What kind of API are we talking about?

Bodigrim avatar May 25 '24 17:05 Bodigrim

The simplest. Something like:

prependByteStringBuilder :: Int -> BB.Builder -> Buffer %1 -> Buffer
appendByteStringBuilder :: Buffer %1 -> Int -> BB.Builder -> Buffer

where the Int is the max size. Other possibilities:

  • use no max size: reallocate if necessary at each build step.
  • use Maybe Int: mix of the two previous.

Reading the Double code more carefully, I realized we still allocate a buffer with BBI.newBuffer. If I understood correctly, there is no way to escape this safely because of the GC, except if our Buffer uses a pinned array.

wismill avatar May 26 '24 04:05 wismill

I'm not sure that tying us that close to internal details of BB.Builder is a good idea. They can change and they do not quite spell out how to use them safely. Unless you are microoptimizing and cutting constant overhead (which is the case for Double builder), you can just run BB.Builder to get ByteString and append it to our builder with fromAddr. For builders doing any significant amount of work the overhead would be negligible.

Bodigrim avatar May 26 '24 18:05 Bodigrim

You mean something like:

appendBSBuilder :: BB.Builder -> Buffer %1 -> Buffer
appendBSBuilder builder = unsafeDupablePerformIO
  (B.unsafeUseAsCString
    (BL.toStrict (BB.toLazyByteString builder))
    (\(Ptr addr) -> pure (|># addr)))

appendBSBuilder' :: Buffer %1 -> BB.Builder -> Buffer
appendBSBuilder' buf builder = foldlIntoBuffer
  (\b bs -> unsafeDupablePerformIO (B.unsafeUseAsCString bs (\(Ptr addr) -> pure (|># addr))) b) 
  buf
  (BL.toChunks (BB.toLazyByteString builder))

This does not look trivial. If we do not provide an API, it would be a good idea to document the best method to implement it.

wismill avatar May 26 '24 19:05 wismill

Yeah, something like this. Documenting it or maybe even providing helpers similar to ones you just wrote would be nice.

Bodigrim avatar May 26 '24 19:05 Bodigrim