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

Removing the dependency on blaze-builder #108

Open
sjakobi opened this issue Mar 12, 2018 · 4 comments
Open

Removing the dependency on blaze-builder #108

sjakobi opened this issue Mar 12, 2018 · 4 comments

Comments

@sjakobi
Copy link

sjakobi commented Mar 12, 2018

In case you've tried building this package with GHC-8.4 already you may have noticed that blaze-builder isn't compatible yet. While most of blaze's functionality is also available directly from bytestring (or bytestring-builder for older GHCs), Blaze.ByteString.Builder.HTTP isn't. So I've extracted that module into a new package bsb-http-chunked.

While trying to remove the dependency on blaze-builder from http-streams, I've noticed that in several places you construct strict bytestrings from builders, which is a rather expensive operation. I was wondering if you should switch to using a different bytestring builder there. These benchmarks suggest using the bytestring-strict-builder or bytestring-tree-builder package.

I'm not sure whether I'll find the time to finish a PR implementing my suggestions but I thought I'd let you know anyway. :)

@istathar
Copy link
Member

I'm happy to change the Builder to something else, but "this or that or the other thing" implies a degree of investigation and benchmarking. Can you suggest somewhere I can learn about the current best suggestion about ByteString Builders?

(Sorry, I must have missed the discussion about why blaze-builder a bad idea?)

I've noticed that in several places you construct strict bytestrings from builders, which is a rather expensive operation.

I'm not quite sure I follow. You're suggesting we do it in too many places? The content needs to be sent out, so it's going to need to be forced somewhere. We should only be reducing the Builders to a ByteString once, though. Anywhere in particular standing out where we've used Builders wrong?

@sjakobi
Copy link
Author

sjakobi commented Mar 19, 2018

Can you suggest somewhere I can learn about the current best suggestion about ByteString Builders?

The best ressource that I am aware of is this benchmark of various bytestring builders. There are links at the bottom to some sample results. Alternatively you can run the benchmarks yourself.

(Sorry, I must have missed the discussion about why blaze-builder a bad idea?)

When I opened this issue there was quite some brouhaha about blaze-builder breaking a lot of other packages' builds with GHC-8.4.1. The maintainer hadn't responded to requests for a new release for months, so in the end the previous maintainer made a release. During that process several packages switched to using the bytestring API directly.

As I depend on snap-server, and the snap-server testsuite depends on http-streams I wanted to raise the idea of moving away from the then incompatible blaze-builder with you.

I've noticed that in several places you construct strict bytestrings from builders, which is a rather expensive operation.

I'm not quite sure I follow. You're suggesting we do it in too many places? The content needs to be sent out, so it's going to need to be forced somewhere. We should only be reducing the Builders to a ByteString once, though. Anywhere in particular standing out where we've used Builders wrong?

Oh, I didn't quite think this through. Depending on how much data you send out via chunked transfer vs. how much you force into single packets, there may not even be that much advantage to using a builder type that optimizes for constructing a single strict bytestring.


On a related note, may I ask you for a comment on lpsmith/blaze-builder#18? This is about a potential bug in chunkedTransferEncoding.

@istathar
Copy link
Member

I've had a go swapping out blaze's Builder for the one in bytestring, but I've immediately run into wondering where I'm going to get chunkedTransferEncoding and chunkedTransferTerminator from. I guess I'll have to port the code from Blaze.ByteString.Builder.HTTP? Also, do you know where flush is?

@sjakobi
Copy link
Author

sjakobi commented May 29, 2018

where I'm going to get chunkedTransferEncoding and chunkedTransferTerminator from

I've extracted these functions into http://hackage.haskell.org/package/bsb-http-chunked.

flush is here: http://hackage.haskell.org/package/bytestring-0.10.8.2/docs/Data-ByteString-Builder-Extra.html#v:flush

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