[swift-server-dev] HTTP API Sketch v2

[Johannes Weiß]

Hi,


> On 7 Apr 2017, at 13:18, Helge Heß via swift-server-dev <swift-server-dev at swift.org> wrote:
> 
> On 07 Apr 2017, at 13:34, Johannes Weiß <johannesweiss at apple.com> wrote:
>>>>> - maybe such:
>>>>> 
>>>>> func writeTrailer(key: String, value: String)
>>>>> 
>>>>> should be
>>>>> 
>>>>> func writeTrailer(key: UnsafePointer<CChar>,
>>>>>                   value: UnsafePointer<CChar>)
>>>>> 
>>>>> this also works with strings out of the box while
>>>>> allowing applications not using Strings for protocol
>>>>> data ;->
>>>> 
>>>> will work with string _literals_ not Strings, maybe we should have both?
>>> 
>>> It works with any String:
>>> 
>>>  func printIt(_ key: UnsafePointer<CChar>) {
>>>    let s = String(cString: key)
>>>    print("key is \(s)”)
>>>  }
>>>  var abc = “hello”
>>>  abc += “ world”
>>>  printIt(abc)
>> 
>> you're right, my bad! However that's not cheap because it will need to copy the String. A C string is \0 terminated contiguous memory and a Swift string might not be \0 terminated in memory so it'll need to make a copy which is not ideal.
> 
> I can’t follow you here. Eventually the String has to be converted to a socket buffer. Since String doesn’t have an API to walk the segments (right?) you always end up doing the thing above. (unless you propose filling the socket buffer by walking the utf-8 sequence ;-)

well, AFAIK String stores a backing buffer plus length in bytes. So there's not necessarily a trailing \0 stored there. But to expose it as a CString it will need to 'add' that and hence probably copy it.


> For practical purposes most header values are very likely cstring-backed in the first place, right?

No, from http_parser.c you'll get back a char * pointer and length, there won't be any \0 bytes unless you make an extra copy.


>>>> Both have a `custom` case now for everything else. And most use-cases would use higher-level frameworks anyway.
>>> 
>>> Well, ideally a framework could reuse common stuff like the HTTPMethod type. It looks a little stupid to wrap such :-)
>> hmm, I'd wrap it to not export low-level implementation details.
> 
> OK, one last try :-): The more types (especially so low level ones) can be successfully shared between higher level libraries,
> a) the more consistent the S3 UX is for users and
> b) 3rd party package authors which can restrict themselves to this protocol set, only need to build a single implementation.
> 
> 
>>>>> - Talking enums. You use them for status and method,
>>>>> why not for header keys? I see _big_ performance
>>>>> advantages in that.
>>>> that's surprising, what exactly is giving you the big advantages and how much is big? Is it the storage or what exactly?
>>> 
>>> - storage (essentially a byte vs the full String thing)
>>> 
>>> - storage reuse (are you going to reconstruct the String
>>> every time you parse a header? or have a thread safe
>>> intern map?) No issue with a byte.
>>> 
>>> - ARC (potentially, much more likely than for Ints ;-)
>>> - comparison speed - compare two bytes (a single
>>> assembly instruction) vs comparing two Strings,
>>> potentially unique ones with all the checks involved.
>>> Order of magnitude more assembly instructions (I guess ;-)
>> 
>> that can all be implemented in HTTPHeaders as an implementation detail, right?
> 
> Hm, no? If you have to construct the Strings for the HTTPHeaders you gain nothing. It actually is worse because do the work twice :-)
> 
> The parser could already match the headers (at the C level, via strcmp() ...) and pass up only enums. Or even better, the state machine of the http_parser could be changed to cover more standard headers.

well, there could be a `mutating func addHeader(key: UnsafeBufferPointer<CChar>) method on HTTPHeaders which compares the key using strcmp and if it sees that it's 'Host' for example it reuses a shared "Host": String for all instances or even an enum or so.


> Also: Why not use Strings for method then? Would be more consistent and would also avoid the protocol-changes issue ;->

could do


>>> I’d like to mention again that in HTTP/2 methods are just headers. Maybe the stuff should be modelled after that?
>>> 
>>> request[.method] == .put
>> 
>> I do believe that people expect a request method field for HTTP/1.
> 
> The end user? He also usually expects a contentType field. Here you go:
> 
>  extension HTTPRequest {
>    var method      : HTTPMethod { return request[.method]      }
>    var contentType : MIMEType   { return request[.contentType] }
>  }
> 
> But I don’t care too much, just an idea to line things up with the future.

that makes things less type-safe than they can be but I'm also not too fussed.

-- Johannes