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/