

Boost :

Subject: Re: [boost] [review][beast] Review of Beast starts today : July 1 - July 10

From: Niall Douglas (s_sourceforge_at_[hidden])

Date: 2017-07-08 15:06:14

I have seen enough reviews from others to be able to finish my review.

This is my formal review of the submitted Beast library.



> - What is your evaluation of the design?



It depends from where you are approaching.



As a niche Boost library of the kind most recent entrants to Boost are,

it's of good quality, solves a need, docs are good, testing is good,

implementation is not bad. One could argue it stops too far short for a

niche library, and it should fill in some of the obvious gaps as most

niche libraries do. But if accepted as so far most of the reviews

recommend, I think it'll fill out with time. It's not a concern for

acceptance.



As a fundamental essentials Boost library with potential for

standardisation (which the author has stated is the case, and I agree),

I found a lot of problems.



To clarify what I mean by "fundamental essentials", the really profound

thing about HTTP is that whenever you invent some mini request-reponse

protocol with tagged parameters and structured payload for some use

case, you inevitably end up reinventing HTTP. It's quite uncanny in

fact, almost disturbing, how often that wheel has been reinvented, and

usually very poorly. I have encountered so many lousy, low quality,

insecure, broken, memory corrupting, reinventions of HTTP in my career

it is quite depressing.



Hence there is a huge need for a properly written, fuzz tested,

low-level, lightweight, generalised request-response structured and

tagged data framework for C++ (and all languages) which is very widely

useful. Such a framework would have a very good chance of getting

standardised because it would be so damn useful for such a wide variety

of use cases, including traditional HTTP-over-socket, but also config

files, metadata, debug printing and so much more. Anything not so

general is correspondingly harder to standardise because I believe my

observations on this topic to be widely held and agreed with once people

think about it.



Why Beast falls far short of a fundamental essentials Boost library:



1. There is a lack of a well defined formal interface separating the

structured data parts from the i/o parts. This is never a good sign.



2. There is a lack of a well defined formal model abstracting away the

data parsing and generation model from a duplex stream model. Gavin said

a stream i/o model is a good one to target, I disagree. It is a

reasonable first but *wrong* guess to target, but in fact once you've

done a few of these you realise it is not optimal. Let me explain.



A generalised low-level request-response vocabulary library should not

impose a stream i/o model as there are so many widely used non-duplex

non-stream i/o models out there in use. In the past for example I've had

available to me a low bandwidth low latency transport and a high

bandwidth high latency transport. It makes huge sense to send the HTTP

headers via the low bandwidth low latency transport and the HTTP bodies

via the high bandwidth high latency transport. Another very common use

case is the ultra-high-latency transport better known as the file

system: how many times have people reinvented variations of

request-reponse protocol with tagged parameters and structured payload

now? INI, JSON, XML, TOML, YAML - they're all just a set of tagged

parameters and structured payload where request = read() and response =

write() over a non-duplex ultra high latency transport.



Those sorts of use case are easy implement with a generalised

"view/range of blobs of bytes" i/o model as I had proposed which is most

obviously implemented in C++ using a mix of span<T> and the Ranges TS.

It's much harder to implement, and with no corresponding gain either in

ease of use, with hard assumption of a duplex scatter-gather stream i/o

model. By hard coding the use of ASIO's inflexible and niche buffer

infrastructure, it makes Beast unusable as fundamental essentials

library, even if WebSocket, ASIO and all that other unnecessary stuff

were removed.



3. Minimum overhead design principles are not followed (zero copy, zero

malloc, zero blocking). The proposer will no doubt now aggressively

shout at me and harass me with "prove it in code". But I have given up

on trying to persuade him of anything, he makes it too unpleasant and mean.



Therefore as a fundamental essentials Boost library, Beast as proposed

is *not* generally useful because of how unnecessarily limited its use

cases are. If it were being reviewed as a fundamental essentials

library, it would have to be a REJECT with an encouragement that he's

done a good first attempt, but needs to think again.



> - What is your evaluation of the implementation?



The implementation is generally of high quality and I came away positive

from scanning the source code. Too much memory allocation, too much

memory copying for my taste, but it is entirely possible that that is

not important relative to the cost of implementing HTTP-over-socket anyway.



I had intended to run some comparative benchmarks between Beast and some

private code I wrote for clients here which uses all minimum overhead

design principles to see how much of a difference there might be. But

when the author made it clear he would not consider a departure from

being hardcoded to ASIO, I decided a further investment of my time here

would not be worth it, and it would require a fair few hours which are

better invested into Outcome v2 which is to be announced as feature

complete on Monday.



So all I can say is that the implementation is good enough for a niche

Boost library.



> - What is your evaluation of the documentation?



Pretty good. I got up to speed quickly. If the unnecessary parts of

Beast were removed and thus the documentation trimmed in size, it would

be better again.



> - What is your evaluation of the potential usefulness of the

> library?



No doubt it solves well its niche use cases.



> - Did you try to use the library? With which compiler(s)? Did you

> have any problems?



I was going to, but decided not to bother given that the author would

not be receptive to any additional effort invested by me.



> - How much effort did you put into your evaluation? A glance? A

> quick reading? In-depth study?



About six hours. I did look at the library in detail back when

Boost.Http was being developed under GSoC, but I've not been paying much

attention since.



> - Are you knowledgeable about the problem domain?



Very. I have worked several contracts where a lightweight subset of HTTP

was implemented using all sorts of unusual non-socket transports. I have

been working on the ultimate low level foundation layer for all future

C++ i/o since 2012, and I have given very deep thought on how best to

implement minimum overhead i/o on very low latency hardware devices,

some of which has been discussed over several years with broad general

agreement as to my approach with various WG21 members, some very senior.



> Thank you for your effort in the Boost community.



Before I give my formal recommendation, I wish to preempt the inevitable

angry response to this review by telling the author that he did a good

library, but he's done a lousy review. One of the very hardest, most

valuable, rarest thing you can obtain in any knowledge based community

is high quality useful feedback on what's possible and desirable by

experts in their fields. That sort of high quality advice costs an

enormous sum of money to rent in, yet you can get it for free here if

you play your cards right. Had the proposer come here looking to make

the ultimate low level HTTP library, he could have come away with a

wealth of useful ideas of what's possible.



Instead he came here with a very narrow goal of getting his existing

library, tolerating only very minor changes, into Boost. And that's

okay, it's a fine niche library, but such a wasted opportunity on what

he could have achieved instead. But his loss in the end.



With all that being said, the final question to answer is whether Beast

is a niche Boost library or a fundamental essentials Boost library?



My own personal opinion (as is obvious above) sways towards fundamental

essentials. After all, it's supposed to be a *low-level* HTTP library.

But the wider community, based on the content of reviews to date, appear

to be thinking niche. I had a feeling that that would be the case, and

that's why I wanted to wait until enough reviews came in for me to be

able to call it.



Following the majority opinion that Beast is a niche not a fundamental

essentials Boost library, I therefore vote to ACCEPT the library without

conditions. It's a good library and a valuable addition to Boost. Well

done Vinnie!



Niall



-- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/

Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk