On 28 Aug 2015 at 12:31, Andrey Semashev wrote:
On 28.08.2015 05:28, Niall Douglas wrote:
The final example is over 1000 lines of code. I didn't think it mattered hugely to the review.
You mentioned this earlier as well. I keep wondering why an example requires more than 1000 lines of code? Is it because the task is too broad or because the library interface is too verbose? In both cases something should be done about it - either change the docs or the library.
The earlier steps in the tutorial showed a design probably close to what the average C++ programmer might first think of and how you might implement that in STL iostreams and then AFIO. All those steps are complete, and with benchmarks. The final step shows a design that an expert in the field would write as an example of a real world solution to a real world problem as a comparator to the earlier examples. Similar to lock free memory programming, the final step is entirely file system lock free and uses numerous tricks of the trade to achieve a superb technical solution to the original problem of a simple key value store. It contains memory mapped dense hash maps (alone is about 250 lines of implementation), a concurrent transaction conflict resolution mechanism, resilience to incomplete and/or partial writes and corrupted metadata including power loss, and a complete versioning system letting you replay the history of transactions modifying the store. It achieves late durability without requiring fsync by making use of concurrent atomic appends to enforce extent aging and flushing, so performance is really excellent compared to alternative designs. The fact all that fits into just 750 lines (1000 - 250 for the dense hash map) is a demonstration of just how much utility and time saving AFIO delivers. But for sure it's going to be gobbledy gook to anyone not versed in the field which is precisely why it both should be in there and isn't important for a Boost review in my opinion. Do note that the documentation *does* describe the final stage in detail and how it works. I felt that level of detail sufficient for a review, though the comparative benchmarks would have been nice. As I mentioned, I had a hard drive failure which got in the way. It happens.
Calling std::terminate is also something I would call unexpected and harmful. You don't know what the application is doing - one thread may be doing something with Boost.AFIO and another may be doing domething important and unrelated. Throw exceptions on errors, where possible. Document UB when not.
If AFIO is fatal exiting the process, you are in an unrecoverable cannot continue situation. I mainly have those traps in there for handling memory corruption which is not uncommon when writing file system applications. They are *extremely* useful for catching inadvertent memory corruption. And they should never trip unless memory corruption has occurred or you have a serious error in how you are using AFIO.
Or we are about to deadlock, and again the user probably wants to know why their application has just hanged itself.
Let it deadlock. Seriously. At least the user will be able to see it blocked and get a backtrace to figure out how did it come to this. Terminating the app is the most useless and harmful reaction.
Fatal exit happens when AFIO cannot continue without losing or corrupting other people's data. I make no apology for that - it is the right thing to do. Deadlocks are handled by continuing to deadlock in debug builds, though with plenty of help and backtraces etc printed to std::cerr. This lets you interrupt in a debugger and try and figure out the cause. In release builds, after a reasonably long timeout you get a terse single line message to std::cerr and it breaks the deadlock and mops up the orphaned state as best it can before continuing. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/