close
Skip to content
This repository was archived by the owner on Nov 23, 2021. It is now read-only.

Update with meeting minutes#52

Merged
seabaylea merged 2 commits intoswift-server:masterfrom
seabaylea:http-nov21-minutes
Dec 7, 2016
Merged

Update with meeting minutes#52
seabaylea merged 2 commits intoswift-server:masterfrom
seabaylea:http-nov21-minutes

Conversation

@seabaylea
Copy link
Copy Markdown
Member

Minutes from Monday's HTTP kick-off meeting.

@swift-server/stakeholders please review.

Comment thread meetings/http/2016-11-21/minutes.md Outdated
### Minutes:

_How are we going to interface these things together, ie, how HTTP sits on networking?_
* Concurrency between now and at least Swift 5 is going to be Dispatch based. From Swift 5 and onwards there may be/will be a new concurrency model. We’ll need to transition at that time.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the will be is unnecessary here, since it is covered by the may be. In any case the high level goals for Swift 5 have not yet been debated, let alone agreed, so it would be incorrect to conclusively state that they are part of it.

Comment thread meetings/http/2016-11-21/minutes.md Outdated
_How are we going to interface these things together, ie, how HTTP sits on networking?_
* Concurrency between now and at least Swift 5 is going to be Dispatch based. From Swift 5 and onwards there may be/will be a new concurrency model. We’ll need to transition at that time.
* Assuming Dispatch gives us the same problem as elsewhere when migrating to a new concurrency model, so we share the problem/solution
* HTTP APIs should/will be available separately from networking/concurrency, as well as a combined.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should/will is unnecessary - either use should, or use will.

Comment thread meetings/http/2016-11-21/minutes.md Outdated
* HTTP APIs should/will be available separately from networking/concurrency, as well as a combined.

_What model are we going to use? Should request/response be value types or reference types?_
* Hit a few issues over models, and disagreements on the HTTP models. Reference vs. value types for the request class.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Hit a' is missing a subject - who hit them?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds to me like one could replace "Hit" with "Came across"

Comment thread meetings/http/2016-11-21/minutes.md Outdated
* There’s some lessons learned from the openswift / swiftx experience


_HTTP 1 vs HTTP2 support_
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have a space between HTTP 1 but not HTTP2? Should be consistent.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historical precedent

Comment thread meetings/http/2016-11-21/minutes.md Outdated

_Reference vs. value-types_
* (Logan) Vapor had success with reference types rather than value types
* (Shmuel) Kitura has issues moving NSData -> Data. Middlewares work better with reference types
Copy link
Copy Markdown
Contributor

@alblue alblue Nov 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has or had? I thought the migration had been completed? Also, why is middlewares plural?

Comment thread meetings/http/2016-11-21/minutes.md Outdated

_Use cases:_
* HTTP on server only? What about on the client for device -> device?
* Foundation has a dependency on libcUrl, we’d ideally help to allow that to be removed.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libcurl doesn't have any capitals

Comment thread meetings/http/2016-11-21/minutes.md Outdated
* At the lowest level, we’ll just be providing bytes - the framework can deal with the contents?

_Use of C vs pure Swift parsing:_
* Swift “port” of the c-http parser: as close to the C version as possible - line for line translation. 700K requests/sec with C, vs, 500K for Swift.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the node c http parser? Not clear from the notes. Also worth noting whether the 500k time was due to reference counting, and if so, whether Ben C's comment regarding whether or not the performance could be improved internally using Unsafe*Pointer instead of passing references might have changed that outcome? Is the source for this attempt available anywhere for others to review?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the port to the Node.js C parser that was discussed:
https://github.com/smithmicro/HTTPParser

Other comments

  • UnsafePointer is already being used
  • Significant time is still being spent in reference counting
  • We discussed during the call that it would be good if an expert in Xcode Instruments to review the codebase. I will send an email to the reflector later today on this topic.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also says:

Using pure-Swift enables deeper level of inlining, so may ultimately give greater performance

this one (and my port too) uses a lot of functions while the upstream C one almost exclusively uses macros aka inlines everything from the beginning. So while the Swift compiler may have better inlining of functions, the C one actually inlines everything already :-)
I tried to replicate that by using the unofficial @inline(__always) which improves perf quite a bit. BUT it has also been very fragile, i.e. the Swift 3 compiler would often just crash etc.

I setup a performance test for my parser Swift port in summer: Performance Comparison of Swift http-parser port. And played a while with the port to improve the speed. I need to add the SmithMicro parser to that.

You may find some tricks in there to further improve the SmithMicro perf, a collection:

  • @inline(__always) is one.
  • Another one is that it uses an Array for tokens and unhex, using a ptr is faster (let tokens : UnsafePointer<CChar> = copyArrayToBuffer(array: tokensArray)).
  • use a struct not a class for http_parser
  • http_errno is implemented as a String enum, that should be just an enum

A major advantage of the SmithMicro port seems to be that it uses try/catch to emulate gotos (I used inline functions which seem to be way worse :-)

Comment thread meetings/http/2016-11-21/minutes.md Outdated

_What model are we going to use? Should request/response be value types or reference types?_
* Hit a few issues over models, and disagreements on the HTTP models. Reference vs. value types for the request class.
* There’s some lessons learned from the openswift / swiftx experience
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we mention what specifically the lessons were?

* Foundation has a dependency on libcUrl, we’d ideally help to allow that to be removed.


_Reference vs. value-types_
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need we decide this at all? We might get pretty far with just a protocol and the appropriate protocol extensions?

The model for Edge would work much better with value types since operations are immutable mappings. It's hard to say that one should be chosen over another.

_Use of C vs pure Swift parsing:_
* Swift “port” of the c-http parser: as close to the C version as possible - line for line translation. 700K requests/sec with C, vs, 500K for Swift.
* Using pure-Swift enables deeper level of inlining, so may ultimately give greater performance
* Using Swift code may also enable a wider set of contributors/maintainers for the code from the swift community (vs. the existing community working on the C code).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, it is a win if it's something the Swift community needn't maintain at all.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't matter whether the cat is black or white, as long as it catches mice。

@seabaylea
Copy link
Copy Markdown
Member Author

I've tried to merge in everyones feedback and comments where possible (not always in the same level of detail). Any further discussion is best done via the mailing list.

@seabaylea seabaylea merged commit 773840f into swift-server:master Dec 7, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants