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

hPutBuilder #586

Closed

Conversation

BebeSparkelSparkel
Copy link
Contributor

@BebeSparkelSparkel BebeSparkelSparkel commented Apr 27, 2024

closes #446

Questions

  1. If there is no encoding specified in haCodec, should should hPutBuilderUtf8 be used?
  2. Does the write byte buffer actually need to be flushed?
  3. Is the ByteArray in danger of being garbage collected leaving a dangerous pointer?
  4. What should the buffer size be? Or, should it be a parameter?
  5. With the change to Builder should toLazyTextWith still use smallChunkSize?
  6. Since the buffers are lazy should there be some pre-sequencing of the next buffer. If so, how could this be done?

@BebeSparkelSparkel BebeSparkelSparkel marked this pull request as ready for review April 30, 2024 15:52
@BebeSparkelSparkel BebeSparkelSparkel marked this pull request as draft May 1, 2024 21:05
@BebeSparkelSparkel BebeSparkelSparkel marked this pull request as ready for review May 5, 2024 17:00
@Lysxia
Copy link
Contributor

Lysxia commented May 27, 2024

I don't see how you can do much better than hPutBuilder h b = L.hPutStr h $ toLazyText b because Builder has to construct a list of chunks anyway (as you mentioned yourself in the linked issue).

The only place I can see a potential speed up is if the desired encoding is UTF-8, then you could go through Text.Utf8.hPutStr, whose implementation could be improved by removing the encodeUtf8 copy and writing the Text bytes directly to the handle.

text's Builder's only purpose is to produce a LazyText, so I think it makes sense to keep its current API minimal and not even add hPutBuilder. Note that it's a quite different data structure from bytestring's Builder, which doesn't allocate buffers itself. Converting Text to bytestring's Builder may be a better idea than going through text's Builder.

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

Successfully merging this pull request may close these issues.

Add hPutBuilder for Text's Builder type
2 participants