aesiniath / http-streams

Haskell HTTP client library for use with io-streams
https://hackage.haskell.org/package/http-streams
BSD 3-Clause "New" or "Revised" License
50 stars 48 forks source link

Removing the dependency on blaze-builder #108

Open sjakobi opened 6 years ago

sjakobi commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 https://github.com/lpsmith/blaze-builder/issues/18? This is about a potential bug in chunkedTransferEncoding.

istathar commented 6 years ago

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 commented 6 years ago

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