Re: [boost] [http] Formal review of Boost.Http

On Thu, Aug 13, 2015 at 9:53 AM, Vinícius dos Santos Oliveira < vini.ipsmaker@gmail.com> wrote:

2015-08-11 17:26 GMT-03:00 Rodrigo Madera :

...

[...]

Many of the Rodrigo Madera's concerns were addressed by the design choices chapter. I'm glad this chapter proved to be useful, so I don't need to repeat myself and users can lose their doubts without even leaving the Boost.Http documentation.

I'm actually challenging some of those answers. Please read again. The library, if accepted as is, doesn't have a guarantee that you will change it back to C++98/03. Same goes to a header-only design. But you're right. Let's focus on the design and implementation details for now. Boost.Http is done from the very beginning to allow alternative backends

(like I stated on my reply to Niall just a few minutes ago, with much more points tackled). Some backends might not even do network or need Asio (shared memory among process or other means). This feature is important for HTTP applications.

OTOH, Boost.Http is fully async and uses Asio abstraction to do

asynchronous code. I've already played with several asynchronous frameworks that use different approaches (Qt, Node.js, futures...) and I'm very impressed by Asio proposal. Boost.Http will always depend on Boost.Asio to abstract its asynchronous nature.

Boost.Http will always depend on Asio, then. So I still see the need to use full Asio abstractions and support for non-Boost Asio. Anyway, to not delay my opinion further. It's not that I'm very against

putting Boost.Http into Asio.Extensions or alike. I'm against putting Boost.Http into a mix of network protocol libraries like FTP, SMTP and others. The reason I'm against is because it may trick the user to think that this is (1) merely a protocol implementation and (2) there will be network activity when Boost.Http doesn't even need network activity (e.g. shared memory based backends).

If you can abstract it for HTTP, I can assume it can be done for others as well. Just take your abstractions and implement them for other protocols. That would still make Boost.Asio.Extensions interesting. The name Boost.Http by itself already proposes the idea of network traffic. Message abstraction with various representations (binary, textual) is a common pattern in networking code. I would expect any other protocol implementations to use it, and if possible, use different backends. cpp-netlib, pion and many other provide request routers and server

abstractions. They usually couple all abstractions together and use a design like:

- There is the server+request-router+handler+thread-pool+... - Request factory, which the user will extend and pass to the above inflated abstraction.

But this is good for a high-level API. As I stated, it would be nice to have both APIs. Have a lower level API and then an abstraction. Since you use Asio, you won't need all those details of cpp-netlib, but the rest could be compile time policies into a higher API.

The thing here is that I'm very ambitious about this project and I estimate I'd take as long as the already spent time just to deliver the request router. I want to be sure that I'm providing the best request router possible to write. Something that could go into the C++ standard and nobody would fear to be deprecated in the future.

Good! You should be. But we need it now to use Http in a real world application, or you will be leaving to users the responsibility of implementing Http code that should be inside the library already. Even if not the perfect solution at first.

A big challenge to the request router is that it can become rather inefficient if I put customization points everywhere. And if I think about performance, I remember that most of the request routers I saw allow you to change the rules at runtime when this feature is rarely needed. Maybe some TMP magic can help here.

Could you please elaborate why? As I see it, if you put the customized interceptors in place, only (and only) that request/response would "suffer". I agree that runtime rules are seldomly needed. In my experience, at least. - Benchmarks

...

I'd suggest to focus on correctness. It's very tricky to get a correct implementation that will portably work among several little know implementations.

What if the design is very far from others at the time? What if the basic Asio examples (with some processing that actually suffices some use cases) outperform severly? Since you have several competitors, benchmarks, to me, are important. Even if you design a perfect API with every abstraction possible, I wouldn't use it if it cut my TV streaming server performance even by a small percentage. It wouldn't be an advantage. I want to be sure that the abstraction won't cost me more than I'm willing to pay. And theoretically, there is no reason why Boost.Http it should underperform if enough static customization is allowed. You just should not use an unmaintained copy & paste

solution.

Sorry, I didn't understand. Do you mean to copy and paste benchmark code? - HTTP 2.0

I'll try to provide a proof-of-concept tonight. It won't be Boost quality and I think I'll use nghttp2. But should be useful to help the job of the reviewers.

Ok, so Boost.Http will have HTTP 2 support itself. Ok. I would love it if the syntax would remain (almost?) identical. - Haste to Review

I'm sorry if I left you guys under this impression. The first answer we got from the review was Niall's reply, focusing on features that would not even go on the Boost code or change the API design. This fact might have contributed to this view. These features aren't required for Boost and it'd be very weird to have a library rejected because these requirements.

We have been talking sporadically about your project since it was approved in GSoC, remember? I raised you my concerns on private messaging as well, way before the review started. While I agree with some points of Niall, I base my opinions on my own use cases. I download the tree, I compile it (when needed), read the examples, do the tests, and then raise my view. I'm not even done playing with Boost.Http. I see if I would have done it better. I see if it would make my work simpler. I see the reasoning. Please don't use this "Niall started it" as a shield to unacknowledge the problems pointed out. You didn't even respond to my initial points, disregarding them as someone else's. But if you read carefully, you will see that those are basic and important issues that affect the Boost quality I'm used to.

Also, there are features not implemented now that deserve further explanation to remove the impression that you guys got. This project can accumulate a lot of polemic decisions easily. I worked for quite awhile already and managed to finish the core of the library (and some extra features already like the possibly most amazing file server API that nobody here acknowledged/noticed ;-) ).

I found the documentation not enough for my testing of file_server. I had to PM you to get some example code on its use. If you would first have examples (as many as possible) then it would be better. I want this core into Boost as a confirmation that the design is right so

far. And here I'm defending my design. And maybe the design is right as most (if not all) complaints are unrelated to the already proposed design.

Don't discard points. Tackle them. I'll speed up my POC to make the deadline on my opinions of the library. But please respond to the points I raise. - Conclusion If you only wanted to review design, then a full library inclusion review wasn't needed. I just don't see it done *yet*. Regards, Rodrigo Madera