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

Subject: Re: [boost] [http] Formal review of Boost.Http
From: Rodrigo Madera (rodrigo.madera_at_[hidden])
Date: 2015-08-13 18:17:25

On Thu, Aug 13, 2015 at 9:53 AM, Vinícius dos Santos Oliveira <
vini.ipsmaker_at_[hidden]> wrote:

> 2015-08-11 17:26 GMT-03:00 Rodrigo Madera <rodrigo.madera_at_[hidden]>:
>
> > [...]
> >
>
> 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