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

`HostName` should not be represented by `String` #21

Closed hvr closed 11 years ago

hvr commented 11 years ago

Currently, the following defines exist in Network.Http.Client

type Hostname = String
type Port = Int

which are used by the following functions:

openConnection :: Hostname -> Port -> IO Connection
setHostname :: Hostname -> Port -> RequestBuilder ()
openConnectionSSL :: SSLContext -> Hostname -> Port -> IO Connection

The HostName type should rather be represented by a ByteString as hostnames are not supposed to be Unicode (except for things like punycode) and moreover, ByteString is a more efficient representation. I assume the reason for HostName being String is because the network package's Network.HostName is being mimicked.

PS: on a related matter, it might be sensible to represent Port with something like Word16 or network's Network.PortNumber (which is a newtype around Word16) as that is what valid portnumbers are for TCP/{IPv4,IPv6}

istathar commented 11 years ago

I assume the reason for

It was String because a) that's what the long thread on haskell-cafe concluded hostnames should be, and b) because that's what Network.Socket's getAddrInfo takes (yes, care of the Network.HostName typedef).

So I'm quite happy to change the public signature to a ByteString, but it's still going to cost the String being allocated.

on a related matter, it might be sensible to represent Port with something like Word16

As long as literals still work, I'm ok with that.

Will it actually save 2 bytes, or does GHC end up aligning structures on machine words like C does for structs?

AfC

hvr commented 11 years ago

Andrew Cowie notifications@github.com writes:

[...]

on a related matter, it might be sensible to represent Port with something like Word16

As long as literals still work, I'm ok with that.

yes, literals work as soon as you provide a Num instance (Network.PortNumber provides that too)

Will it actually save 2 bytes, or does GHC end up aligning structures on machine words like C does for structs?

it won't save any bytes on the heap w.r.t. using 'Int' (unless you use unboxed vectors maybe), this measure is just for correctness, as the user then can't create nonsensible port numbers (as the Word16 type can't hold/represent them anyway)...

istathar commented 11 years ago

So, is this something we can sneak in with 0.4.1, or does it need to be 0.5.0? This one is the grey area where a recompile should work transparently, right?

AfC

hvr commented 11 years ago

Andrew Cowie writes:

So, is this something we can sneak in with 0.4.1, or does it need to be 0.5.0? This one is the grey area where a recompile should work transparently, right?

Alas, I think it will require a major version bump, as I can easily think of programs which would break :-/

istathar commented 11 years ago

@hvr So be it. Be nice if we have some other API changes to do at the same time...

AfC