Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an API for the ByteString builder #22

Open
wismill opened this issue May 25, 2024 · 5 comments
Open

Add an API for the ByteString builder #22

wismill opened this issue May 25, 2024 · 5 comments

Comments

@wismill
Copy link
Contributor

wismill commented May 25, 2024

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.

@Bodigrim
Copy link
Owner

What kind of API are we talking about?

@wismill
Copy link
Contributor Author

wismill commented May 26, 2024

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.

@Bodigrim
Copy link
Owner

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.

@wismill
Copy link
Contributor Author

wismill commented May 26, 2024

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.

@Bodigrim
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants