1. Should Boost.AFIO be accepted into Boost? Please state all conditions for acceptance explicity.
In its current state, Boost.AFIO should NOT be accepted into Boost. However, I am confident that Niall will be able to deliver a future version of AFIO that should be accepted into Boost after another review. There is a great need for more powerful and efficient file handling functionality in C++ than is provided by iostreams and Boost.Filesystem/Filesystem TS. I appreciate the enormous complexity of trying to define a multi-platform filesystem interface, and it is clear that Niall has done a ton of very valuable research on this topic.
2. What is your evaluation of the design?
A key strength of the design is that it provides a common interface for
asynchronous operations regardless of whether they are internally implemented
using Windows IOCP support, async IO on FreeBSD, or regular POSIX operations via
a thread pool.
Another key strength is that it exposes handle-based (i.e. *at), rather than
path-based APIs where possible in order to minimize race conditions, which tend
to be difficult or impossible to avoid completely with file system operations.
I do have a number of concerns about several aspects of the design, however:
Batch API:
- It is not clear what, if any, purpose the batch API serves. The significant
added complexity of using it would have to be justified by a significant
performance advantage, but it is not at all clear that there is even any
performance advantage at all.
- I suggest removing it unless there is a strong argument otherwise.
File handle API:
- The file handle abstraction in AFIO is very heavy weight, involving a
shared_ptr to a structure containing a whole bunch of fields in addition to
the native file handle, including a mutex, numerous flags, the path used to
open it, a pointer to a handle to the parent directory, a chrono time_point
specifying the time the file was opened, the total number of bytes read,
written, and written since last fsync using this file handle.
- Because file handles are at the core of AFIO, it is critical that users
precisely understand what this abstraction involves, and specifically how it
differs from a native file handle.
- I'd be much more comfortable with a much leaner abstraction, like a
unique_handle type that just contains the bare native handle, and uses unique
ownership semantics. The fat handles impose both computational/memory
overhead as well as mental overhead on the programmer. I understand these
extra fields are probably necessary to provide various functionality,
particularly emulation of behavior provided by other operating systems, but it
would be nicer to be able to opt-in to such functionality and only pay for it
when needed.
- At the very least, if these fat file handles are kept, a very strong argument
needs to be given as a rationale. (Perhaps you have found the overhead to be
negligible relative to the cost of the native file handle.)
- It should be possible to construct an AFIO handle from a native file
descriptor.
- On POSIX, AFIO should support supplying platform-specific open flags, like
O_NOATIME.
- I'd much prefer native_handle() to return the actual appropriate native handle
type for the platform, e.g. int on POSIX, rather than a void *. I'm guessing
this is done in order to support alternative dispatchers, that aren't
necessarily based on OS files at all, in the future, but perhaps a better way
of doing this that doesn't require casting the void* to an int, can be found.
Future-based API:
- I agree with Thomas Heller regarding the Future-based API:
- The implicit future.then built into the API should be eliminated. Instead,
API functions should take a file handle directly.
- future<T> should not implicitly store a file handle. The API functions that
open a file can return a future to a file_handle. API functions that don't
open a new file should just return a future to the actual result.
Inconsistency in how operations are defined:
- Some operations are methods of the dispatcher, some are methods of the file
handle, and others are free functions.
- I suggest that all file system operations be free functions, and take a
dispatcher as an argument.
Thread-local current dispatcher:
- Some operations involve an explicitly specified dispatcher, others seem to
obtain it from the file handle, and others default to a thread-local
dispatcher that has been set.
- Given that it is extremely likely for user code to execute from multiple
threads given the nature of AFIO, relying on a thread-local current dispatcher
seems very error prone.
- I suggest that the thread-local current dispatcher be eliminated, and that the
dispatcher always be specified explicitly to any API call that requires one.
Directory handle cache:
- A cache of open handles to directories is kept by AFIO by default. This is
particularly problematic because (as is documented) it makes reading directory
entries thread-unsafe by default. If this is to be kept, a very strong
rationale needs to be given, and the behavior must be changed somehow to
ensure that directory reading is reliable by default.
Enumerate API:
- The future-based API of async_enumerate seems to be unnecessarily complicated,
due to the need to specify maxitems and deal with making repeated calls, and
all of the copying and memory allocation required to return a std::vector of
directory_entry. I suggest instead something like passing in a
std::function
4. What is your evaluation of the documentation?
- The extensive documentation about race guarantees is extremely useful. - I appreciate the in-depth examples. - Much more design rationale is needed. - The term "filing system" is used in place of the more common "file system", leading me to wonder if there is a difference in meaning. - Every instance where there is a platform-specific #ifdef in an example should be evaluated, so that you can figure out the appropriate platform-independent abstraction that could be added to AFIO in order to move that #ifdef into the library and out of user code. - There is an attempt to document differences in behavior on different platforms, but this is not done sufficiently. For example, it is stated that async_write requires that the destination region does not go off the end of the file, but on POSIX pwrite supports this just fine as a way to extend the file. I assume that this restriction applies to Windows only. - The semantics of the Batch API are not precisely documented, nor is any rationale given for it. For instance, I understand that the operations themselves cannot happen atomically, since that is not supported by operating systems, though this is not clearly stated. It is stated: "Once a batch of input ops has been verified at the point of entry as not errored, you are guaranteed that the batch is atomically scheduled as a whole, unless a failure to allocate memory occurs." Does this means that if the *_req object involves futures, it waits for all of them to be ready before launching any of the operations in the batch? Is that the purpose of the Batch API, to do a wait_all on all of the relevant futures, then to launch each operation? This issue goes away though if the Batch API is eliminated, as seems to make sense. - It would be useful to document for each operation which underlying system calls it invokes on each platform. - Many operations other than open (e.g. rmfile) take file_flags, but it is not specified which flags are supported. - References to historical versions of AFIO (prior to the review) should be removed from the main parts of the documentation, since they only distract from the useful information. This information could be kept in a different section at the end for the extra curious. - Formatting of the "description" in the Boost Quickbook format function documentation has some problems: the lack of line breaks between the general description, other notes, and the common note about the use of NT kernel paths make it difficult to read.
5. What is your evaluation of the potential usefulness of the library?
The library is potentially extremely useful. C++ and Boost iostreams and Boost/TS Filesystem provide only very limited functionality. The C library libuv is a close competitor but seems to offer less functionality and a less convenient API. For achieving similar functionality, the status quo in C++ is to rely on platform-specific C APIs that are much more cumbersome to use.
6. Did you try to use the library? With what compiler? Did you have any problems?
I tried some very simple usage of the library with GCC 5.2.0 on Linux. There were a few warnings (variable set but not used) with -Wall that would be best eliminated. - The error messages are not ideal: for instance, in the case that there was an error opening a file, this is the result: Dynamic exception type: boost::system::system_error std::exception::what: File '/home/jbms/tmp/afio-test/test.txt' not found [Host OS Error: No such file or directory (2) in '/home/jbms/tmp/boost.afio/include/boost/afio/v2/detail/impl/afio.ipp': async_io_handle_posix:805]: No such file or directory. Op was scheduled at: 1. 0x42fb4f: ./test (+0x2fb4f) : No such file or directory Some of this is only useful when debugging AFIO, some only useful when debugging the application, and there is a lot of redundancy, for instance the actual error "No such file or directory" is shown in 3 places. Ideally there would be a way to get a simple message along the lines of "Error opening file '/home/jbms/tmp/afio-test/test.txt': No such file or directory" that could be shown directly to the user. I tried to #define BOOST_AFIO_EXCEPTION_DISABLESOURCEINFO to see if that would make the error message better, but instead that seemed to trigger a bug: Dynamic exception type: std::logic_error std::exception::what: basic_string::_S_construct null not valid - A separate issue is that attempting to read a range that is not present in a file, something that the library should handle elegantly, leads to the program aborting immediately with an error message, rather than allowing the error to be handled: FATAL EXCEPTION: Failed to read all buffers 1. 0x40f0f4: ./test (+0xf0f4) terminate called after throwing an instance of 'std::runtime_error' what(): Failed to read all buffers zsh: abort (core dumped) ./test
7. How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I gave the library a fairly in-depth study, spending at least 8 hours in total. I looked at the documentation fairly thoroughly, looked through parts of the implementation, and tried running a couple small programs that use the library.
8. Are you knowledgeable about the problem domain?
I have a fair amount of experience with asynchronous programming in C++, and I have pretty extensive knowledge of the file system APIs on Linux. As a final note, I'd like to thank Niall for all of the work he has done to get the library this far. A powerful, elegant, cross-platform interface for file system operations is very much needed in C++, I really do want to see (a future version of) this library in Boost and use it myself.
On 31 Aug 2015 at 5:26, Jeremy Maitin-Shepard wrote:
1. Should Boost.AFIO be accepted into Boost? Please state all conditions for acceptance explicity.
In its current state, Boost.AFIO should NOT be accepted into Boost. However, I am confident that Niall will be able to deliver a future version of AFIO that should be accepted into Boost after another review. There is a great need for more powerful and efficient file handling functionality in C++ than is provided by iostreams and Boost.Filesystem/Filesystem TS. I appreciate the enormous complexity of trying to define a multi-platform filesystem interface, and it is clear that Niall has done a ton of very valuable research on this topic.
Thanks.
2. What is your evaluation of the design?
A key strength of the design is that it provides a common interface for asynchronous operations regardless of whether they are internally implemented using Windows IOCP support, async IO on FreeBSD, or regular POSIX operations via a thread pool.
Another key strength is that it exposes handle-based (i.e. *at), rather than path-based APIs where possible in order to minimize race conditions, which tend to be difficult or impossible to avoid completely with file system operations.
I do have a number of concerns about several aspects of the design, however:
Batch API:
- It is not clear what, if any, purpose the batch API serves. The significant added complexity of using it would have to be justified by a significant performance advantage, but it is not at all clear that there is even any performance advantage at all.
- I suggest removing it unless there is a strong argument otherwise.
The batch API may, in the end, not prove to be performance positive for the file:/// URI backend. But I can see it could be of great use for http:// or ftp:// URI backends. Even with a zip:// URI backend, batch write would save a lot of rebuilding the ZIP archive.
File handle API:
- The file handle abstraction in AFIO is very heavy weight, involving a shared_ptr to a structure containing a whole bunch of fields in addition to the native file handle, including a mutex, numerous flags, the path used to open it, a pointer to a handle to the parent directory, a chrono time_point specifying the time the file was opened, the total number of bytes read, written, and written since last fsync using this file handle.
- Because file handles are at the core of AFIO, it is critical that users precisely understand what this abstraction involves, and specifically how it differs from a native file handle.
- I'd be much more comfortable with a much leaner abstraction, like a unique_handle type that just contains the bare native handle, and uses unique ownership semantics. The fat handles impose both computational/memory overhead as well as mental overhead on the programmer. I understand these extra fields are probably necessary to provide various functionality, particularly emulation of behavior provided by other operating systems, but it would be nicer to be able to opt-in to such functionality and only pay for it when needed.
- At the very least, if these fat file handles are kept, a very strong argument needs to be given as a rationale. (Perhaps you have found the overhead to be negligible relative to the cost of the native file handle.)
I really feel the need to strenuously disagree with the notion that AFIO has a "fat" handle. People have no comprehension of the cost of the open()/CreateFile() system call relative to a shared_ptr or any of the other things afio::handle does, and they are fixated on judging AFIO's design choice completely inappropriately as a result. To show what I mean, here is a gist demonstrating how lightweight AFIO's handle is relatively speaking: https://gist.github.com/ned14/affa007440f0568e4aa6 This test simply compares opening and closing a file with the same thing plus a shared_ptr creation and destruction. On Linux: +4.76% On Windows: +1.8%. Before people go "aha!", remember that afio::handle has proper error handling and has to configure a number of other items, including asking the OS for what its "real" path is and other housekeeping. When I said that the overhead of shared_ptr was about 0.5%, I meant it. I benchmarked it before taking the decision.
- It should be possible to construct an AFIO handle from a native file descriptor.
That's already in there: simply subclass afio::handle with your own handle type. I can't allow existing afio::handle to use third party descriptors because they need to be configured very exactly on Windows for AFIO to work correctly. This is less of a problem on POSIX, but it still applies there too. When the handle subclass type is third party, AFIO can recognise the situation and handle it correctly.
- On POSIX, AFIO should support supplying platform-specific open flags, like O_NOATIME.
Hmm, maybe. I can see pros and cons. I've logged it to https://github.com/BoostGSoC13/boost.afio/issues/110.
- I'd much prefer native_handle() to return the actual appropriate native handle type for the platform, e.g. int on POSIX, rather than a void *. I'm guessing this is done in order to support alternative dispatchers, that aren't necessarily based on OS files at all, in the future, but perhaps a better way of doing this that doesn't require casting the void* to an int, can be found.
It's more ABI stability. Casting void * to int is fine.
- Given that it is extremely likely for user code to execute from multiple threads given the nature of AFIO, relying on a thread-local current dispatcher seems very error prone.
- I suggest that the thread-local current dispatcher be eliminated, and that the dispatcher always be specified explicitly to any API call that requires one.
Noted.
Directory handle cache:
- A cache of open handles to directories is kept by AFIO by default. This is particularly problematic because (as is documented) it makes reading directory entries thread-unsafe by default. If this is to be kept, a very strong rationale needs to be given, and the behavior must be changed somehow to ensure that directory reading is reliable by default.
The documentation explains exactly why there is this cache, it's a big win despite the complexity it introduces. The problem of races on directory enumeration on shared directory handles is eliminated if you make the number of entries enumerated in a single shot bigger than the total. In practice, this is not a problem, even on 10M item directories, just throw max_items at the problem.
Enumerate API:
- The future-based API of async_enumerate seems to be unnecessarily complicated, due to the need to specify maxitems and deal with making repeated calls, and all of the copying and memory allocation required to return a std::vector of directory_entry.
If you want race free snapshots (up to the platform specific guarantees), simply make sure maxitems far exceeds any potential contents. std::vector does not copy memory on C++ 11 when returned by value.
I suggest instead something like passing in a std::function
const &next)> callback (possibly replacing expected by the new name for your monad type). An empty next value means end of directory has been reached. Returning true means continue listing the directory, returning false means stop listing the directory.
I didn't do this because we actually do fetch maxitems from the kernel in a single shot. If you ask for 10M items, all 10M items are returned in a single kernel syscall as a single, race free snapshot. Calling some callback per item is therefore not useful. On POSIX, with the current implementation directory enumeration isn't particularly race free past the inode and type, which is still quite useful much of the time. However I had in mind in the future we'd pull the directory inode by hand in a single shot and parse it into the AFIO output. FreeBSD lets you do that without any filing system specific code, unfortunately Linux requires you to understand ext4 inode layouts. My point is that you need to think not in terms of conventional directory enumeration. AFIO will enumerate a 1M item directory faster than a traditional design can do 10,000 items. It's so fast you can afford to fetch more than you need and throw away the excess.
- Operations including rmdir, rmfile, symlink, rmsymlink (as well as async variants) that normally would not necessarily even operate on file handles, actually return a handle_ptr. Perhaps if on some platforms a handle has to be opened, then there could be a way for the user to specify that a handle is desired. Otherwise, it would make sense to just return void or std::future<void> equivalent for async variants.
Already logged.
- The documentation notes that on MS Windows, DOS short names are not supported. I don't have much familiarity with Windows, but if programs generally accept DOS short names when a filename is required, then AFIO should probably support these names as well.
The cost of this support is high, and the benefit very low unless you are likely to run original 16bit DOS programs on files generated by AFIO (note 64 bit Windows no longer can run 16 bit DOS programs). You'll run into long path problems on Windows long before 8.3 name compatibility problems.
- The documentation notes: "Don't do directory enumerations from an x86 binary on a x64 Windows system. This is due to a bug in Microsoft's WOW64 syscall translation layer which Microsoft have decided is wontfix. " Given that a major purpose of AFIO is to provide a portable abstraction that works around OS limitations, AFIO needs to support directory enumeration even in this case (e.g. by detecting WOW64 and using a synchronous call via a threadpool in that case).
This problem is caused by how ASIO's reactor works and is due to code I can't modify in ASIO. It'll go away when I rewrite ASIO's reactor using lightweight futures because I can control the source code.
- It is noted that to do an asynchronous stat you have to use enumerate with an appropriate glob specified, but constructing such a glob may be non-trivial if the path contains characters that are interpreted specially by the glob syntax, and impossible to do in a race-free manner.
On POSIX fnmatch() which is used to implement glob allows escaping of the metacharacters. On Windows the glob metacharacters are illegal in a path anyway.
- The implementation might be better split up into multiple independent files. There are various independent facilities, like secded_ecc, that could be placed in a separate file.
Already logged.
- Although this tends to be a problem with most Boost libraries, it would be nice if the compile times could be reduced, particularly when not in header-only mode. Some libraries like Boost Spirit have an interface inherently based on template metaprogramming, such that long compile times are pretty much unavoidable, but the Boost AFIO interface seems to be mostly non-template based, which suggests that it may be possible to get pretty good compile times.
AFIO if configured to do so can achieve excellent compile times as it's something I've worked hard to optimise: https://boostgsoc13.github.io/boost.afio/doc/html/afio/FAQ/slow_compil e.html
4. What is your evaluation of the documentation?
- The extensive documentation about race guarantees is extremely useful.
- I appreciate the in-depth examples.
- Much more design rationale is needed.
- The term "filing system" is used in place of the more common "file system", leading me to wonder if there is a difference in meaning.
- Every instance where there is a platform-specific #ifdef in an example should be evaluated, so that you can figure out the appropriate platform-independent abstraction that could be added to AFIO in order to move that #ifdef into the library and out of user code.
- There is an attempt to document differences in behavior on different platforms, but this is not done sufficiently. For example, it is stated that async_write requires that the destination region does not go off the end of the file, but on POSIX pwrite supports this just fine as a way to extend the file. I assume that this restriction applies to Windows only.
It's more that pwrite off the end of the file is inherently racy to concurrent read/writers. If you want safe automatic file length extension on writes, open a handle with the append flag and use that. AFIO fatal exits the process if you try to read or write past the current file size, it's a logic error and your program is inherently broken.
- The semantics of the Batch API are not precisely documented, nor is any rationale given for it. For instance, I understand that the operations themselves cannot happen atomically, since that is not supported by operating systems, though this is not clearly stated. It is stated: "Once a batch of input ops has been verified at the point of entry as not errored, you are guaranteed that the batch is atomically scheduled as a whole, unless a failure to allocate memory occurs." Does this means that if the *_req object involves futures, it waits for all of them to be ready before launching any of the operations in the batch? Is that the purpose of the Batch API, to do a wait_all on all of the relevant futures, then to launch each operation? This issue goes away though if the Batch API is eliminated, as seems to make sense.
- It would be useful to document for each operation which underlying system calls it invokes on each platform.
- Many operations other than open (e.g. rmfile) take file_flags, but it is not specified which flags are supported.
Good point. Logged to https://github.com/BoostGSoC13/boost.afio/issues/111.
- References to historical versions of AFIO (prior to the review) should be removed from the main parts of the documentation, since they only distract from the useful information. This information could be kept in a different section at the end for the extra curious.
- Formatting of the "description" in the Boost Quickbook format function documentation has some problems: the lack of line breaks between the general description, other notes, and the common note about the use of NT kernel paths make it difficult to read.
5. What is your evaluation of the potential usefulness of the library?
The library is potentially extremely useful. C++ and Boost iostreams and Boost/TS Filesystem provide only very limited functionality. The C library libuv is a close competitor but seems to offer less functionality and a less convenient API. For achieving similar functionality, the status quo in C++ is to rely on platform-specific C APIs that are much more cumbersome to use.
6. Did you try to use the library? With what compiler? Did you have any problems?
I tried some very simple usage of the library with GCC 5.2.0 on Linux. There were a few warnings (variable set but not used) with -Wall that would be best eliminated.
Logged to https://github.com/BoostGSoC13/boost.afio/issues/112
- The error messages are not ideal: for instance, in the case that there was an error opening a file, this is the result:
Dynamic exception type: boost::system::system_error std::exception::what: File '/home/jbms/tmp/afio-test/test.txt' not found [Host OS Error: No such file or directory (2) in '/home/jbms/tmp/boost.afio/include/boost/afio/v2/detail/impl/afio.ipp': async_io_handle_posix:805]: No such file or directory. Op was scheduled at: 1. 0x42fb4f: ./test (+0x2fb4f) : No such file or directory
Some of this is only useful when debugging AFIO, some only useful when debugging the application, and there is a lot of redundancy, for instance the actual error "No such file or directory" is shown in 3 places. Ideally there would be a way to get a simple message along the lines of "Error opening file '/home/jbms/tmp/afio-test/test.txt': No such file or directory" that could be shown directly to the user.
Those "No such file or directory" were from the stack backtracer. I assume it could not figure out your debug info. Regarding your request, in the lightweight futures rewrite we will exclusively use std::error_code for unexceptional errors, and therefore you will have exactly as you ask.
- A separate issue is that attempting to read a range that is not present in a file, something that the library should handle elegantly, leads to the program aborting immediately with an error message, rather than allowing the error to be handled:
There is no way of gracefully handling this without introducing a lot of complexity to IPC with concurrent readers and writers. Besides, it's a logic error to do i/o outside the file's maximum permitted extent, and this is enforced in all asynchronous file i/o applications far outside AFIO. The reason I chose a fatal exception is that this is not a recoverable error, this is a fundamental mistake in your implementation (or memory corruption) and you need to fix it.
7. How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I gave the library a fairly in-depth study, spending at least 8 hours in total. I looked at the documentation fairly thoroughly, looked through parts of the implementation, and tried running a couple small programs that use the library.
8. Are you knowledgeable about the problem domain?
I have a fair amount of experience with asynchronous programming in C++, and I have pretty extensive knowledge of the file system APIs on Linux.
As a final note, I'd like to thank Niall for all of the work he has done to get the library this far. A powerful, elegant, cross-platform interface for file system operations is very much needed in C++, I really do want to see (a future version of) this library in Boost and use it myself.
Thank you for your review! Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 31.08.2015 19:18, Niall Douglas wrote:
On 31 Aug 2015 at 5:26, Jeremy Maitin-Shepard wrote:
- At the very least, if these fat file handles are kept, a very strong argument needs to be given as a rationale. (Perhaps you have found the overhead to be negligible relative to the cost of the native file handle.)
I really feel the need to strenuously disagree with the notion that AFIO has a "fat" handle. People have no comprehension of the cost of the open()/CreateFile() system call relative to a shared_ptr or any of the other things afio::handle does, and they are fixated on judging AFIO's design choice completely inappropriately as a result.
To show what I mean, here is a gist demonstrating how lightweight AFIO's handle is relatively speaking:
https://gist.github.com/ned14/affa007440f0568e4aa6
This test simply compares opening and closing a file with the same thing plus a shared_ptr creation and destruction.
On Linux: +4.76% On Windows: +1.8%.
Before people go "aha!", remember that afio::handle has proper error handling and has to configure a number of other items, including asking the OS for what its "real" path is and other housekeeping.
Why does the library do that when not asked? Why not collect this data only when the user requests?
When I said that the overhead of shared_ptr was about 0.5%, I meant it. I benchmarked it before taking the decision.
I think others have pointed that performance is not the only issue with shared_ptrs.
- I'd much prefer native_handle() to return the actual appropriate native handle type for the platform, e.g. int on POSIX, rather than a void *. I'm guessing this is done in order to support alternative dispatchers, that aren't necessarily based on OS files at all, in the future, but perhaps a better way of doing this that doesn't require casting the void* to an int, can be found.
It's more ABI stability.
Stability with respect to what? Do you expect native handles to be somehow different on the same platform? Or are you trying to maintain stable ABI across different platforms? Why on earth would you do that?
Casting void * to int is fine.
No, it's not. It's a hack that you impose on the library users. According to [expr.reinterpret.cast]/4 it's not even required to compile, as int may not be large enough to hold a pointer.
Directory handle cache:
- A cache of open handles to directories is kept by AFIO by default. This is particularly problematic because (as is documented) it makes reading directory entries thread-unsafe by default. If this is to be kept, a very strong rationale needs to be given, and the behavior must be changed somehow to ensure that directory reading is reliable by default.
The documentation explains exactly why there is this cache, it's a big win despite the complexity it introduces. The problem of races on directory enumeration on shared directory handles is eliminated if you make the number of entries enumerated in a single shot bigger than the total. In practice, this is not a problem, even on 10M item directories, just throw max_items at the problem.
Will this result in the process having 10M open handles permanently?
Enumerate API:
- The future-based API of async_enumerate seems to be unnecessarily complicated, due to the need to specify maxitems and deal with making repeated calls, and all of the copying and memory allocation required to return a std::vector of directory_entry.
If you want race free snapshots (up to the platform specific guarantees), simply make sure maxitems far exceeds any potential contents.
std::vector does not copy memory on C++ 11 when returned by value.
Correction: when returned with a move constructor or RVO.
- There is an attempt to document differences in behavior on different platforms, but this is not done sufficiently. For example, it is stated that async_write requires that the destination region does not go off the end of the file, but on POSIX pwrite supports this just fine as a way to extend the file. I assume that this restriction applies to Windows only.
It's more that pwrite off the end of the file is inherently racy to concurrent read/writers. If you want safe automatic file length extension on writes, open a handle with the append flag and use that.
AFIO fatal exits the process if you try to read or write past the current file size, it's a logic error and your program is inherently broken.
This is unacceptable behavior IMO. I would expect an exception in such case.
On 31 Aug 2015 at 20:39, Andrey Semashev wrote:
Before people go "aha!", remember that afio::handle has proper error handling and has to configure a number of other items, including asking the OS for what its "real" path is and other housekeeping.
Why does the library do that when not asked? Why not collect this data only when the user requests?
The problem is the race free filesystem support which isn't as simple, unfortunately, as simply using the POSIX *at() functions. You need a minimum of the true canonical path of the fd, and its stat_t struct (specifically the inode, created and modified timestamps). That's at least two additional syscalls just for those, and AFIO may do as many as seven syscalls per handle open depending. You can disable the race free semantics, which are slow, using afio::file_flags::no_race_protection. However AFIO fundamentally assumes that any file system path consuming function is slow and will not be frequently called by applications expecting high performance, so work is deliberately loaded into path consuming functions to keep it away from all other functions. If you have a need to do a lot of file opens and closes and don't care about races on the file system, you are better off not using AFIO. STL iostreams is perfectly good for this as the only OSs which allow async file opens and closes is QNX and the Hurd.
- I'd much prefer native_handle() to return the actual appropriate native handle type for the platform, e.g. int on POSIX, rather than a void *. I'm guessing this is done in order to support alternative dispatchers, that aren't necessarily based on OS files at all, in the future, but perhaps a better way of doing this that doesn't require casting the void* to an int, can be found.
It's more ABI stability.
Stability with respect to what? Do you expect native handles to be somehow different on the same platform?
AFIO's design allows many backends e.g. ZIP archives, HTTP etc. So yes, native handles can vary on the same platform and I needed to choose one for the ABI.
Casting void * to int is fine.
No, it's not. It's a hack that you impose on the library users. According to [expr.reinterpret.cast]/4 it's not even required to compile, as int may not be large enough to hold a pointer.
The native handle on POSIX is stored internally as an int. In native_handle(), it is cast to a size_t, and then to a void *. Please do correct me if I am wrong, but I had thought that this is defined behaviour: int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a); This is certainly a very common pattern in C.
Directory handle cache:
- A cache of open handles to directories is kept by AFIO by default. This is particularly problematic because (as is documented) it makes reading directory entries thread-unsafe by default. If this is to be kept, a very strong rationale needs to be given, and the behavior must be changed somehow to ensure that directory reading is reliable by default.
The documentation explains exactly why there is this cache, it's a big win despite the complexity it introduces. The problem of races on directory enumeration on shared directory handles is eliminated if you make the number of entries enumerated in a single shot bigger than the total. In practice, this is not a problem, even on 10M item directories, just throw max_items at the problem.
Will this result in the process having 10M open handles permanently?
God no, it's exactly to achieve the opposite. Open handle count is severely limited on POSIX platforms, and AFIO engages in quite a bit of complexity to reduce the problem of running into process limits on open fds. If you are careful to keep metadata lookups within the directory_entry::metadata_fastpath() set, having to open each file entry to read metadata can be avoided. Your 10M item directory is therefore enumerated without opening a handle 10M times.
std::vector does not copy memory on C++ 11 when returned by value.
Correction: when returned with a move constructor or RVO.
Which AFIO makes sure of, and I have verified as working.
It's more that pwrite off the end of the file is inherently racy to concurrent read/writers. If you want safe automatic file length extension on writes, open a handle with the append flag and use that.
AFIO fatal exits the process if you try to read or write past the current file size, it's a logic error and your program is inherently broken.
This is unacceptable behavior IMO. I would expect an exception in such case.
AFIO assumes the only reason you are reading and writing past the maximum file size is either a logic error or memory corruption. It cannot tell which is which quickly, so we assume it's memory corruption as your program should never have this logic error. And as I explained before, if AFIO thinks memory corruption has occurred it can no longer know if what it has been told to do is correct or wrong, and therefore not terminating the process could cause damage to other people's data. I do have a very big concern that it is a denial of service attack on anything using AFIO by simply shrinking a file it is using from another process to cause the AFIO using process to fatal exit, so I will be trying to improve on the current situation in the engine rewrite. It's not as easy as it looks if you want to keep reads and writes fast, believe me, but again the ASIO reactor is in my way because that is code I can't control. Once I've written a replacement reactor I am hoping it can track scatter-gather writes a lot better than ASIO can and therefore handle this situation better without adding much overhead for tracking the individual operations. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Tue, Sep 1, 2015 at 3:42 PM, Niall Douglas
On 31 Aug 2015 at 20:39, Andrey Semashev wrote:
Before people go "aha!", remember that afio::handle has proper error handling and has to configure a number of other items, including asking the OS for what its "real" path is and other housekeeping.
Why does the library do that when not asked? Why not collect this data only when the user requests?
The problem is the race free filesystem support which isn't as simple, unfortunately, as simply using the POSIX *at() functions.
You need a minimum of the true canonical path of the fd, and its stat_t struct (specifically the inode, created and modified timestamps). That's at least two additional syscalls just for those, and AFIO may do as many as seven syscalls per handle open depending.
Can you describe in more detail exactly why this information is needed/how it is used?
You can disable the race free semantics, which are slow, using afio::file_flags::no_race_protection. However AFIO fundamentally assumes that any file system path consuming function is slow and will not be frequently called by applications expecting high performance, so work is deliberately loaded into path consuming functions to keep it away from all other functions.
If you have a need to do a lot of file opens and closes and don't care about races on the file system, you are better off not using AFIO. STL iostreams is perfectly good for this as the only OSs which allow async file opens and closes is QNX and the Hurd.
There is certainly a large gap between what is possible with STL iostreams or Boost filesystem and what is possible using platform-specific APIs for file system manipulations. While it is obviously your choice what scope to pick, I think it may be possible for it to be usable for everything you intend to do with it, e.g. for writing a database backend, but also to be much more widely useful.
- I'd much prefer native_handle() to return the actual appropriate native handle type for the platform, e.g. int on POSIX, rather than a void *. I'm guessing this is done in order to support alternative dispatchers, that aren't necessarily based on OS files at all, in the future, but perhaps a better way of doing this that doesn't require casting the void* to an int, can be found.
It's more ABI stability.
Stability with respect to what? Do you expect native handles to be somehow different on the same platform?
AFIO's design allows many backends e.g. ZIP archives, HTTP etc. So yes, native handles can vary on the same platform and I needed to choose one for the ABI.
Perhaps you could provide a less generic API for the native platform filesystem, and then once you also have support for ZIP archives, etc. create a more generic interface on top. While archive files and different network protocols like WebDAV and FTP can certainly be presented as file systems, particularly for read-only access, and indeed it is often possible to access them as file systems through the native platform APIs, they tend to differ sufficiently from file native systems that a different API may be more suitable. [snip]
std::vector does not copy memory on C++ 11 when returned by value.
Correction: when returned with a move constructor or RVO.
Which AFIO makes sure of, and I have verified as working.
What I meant is that returning std::vector
It's more that pwrite off the end of the file is inherently racy to concurrent read/writers. If you want safe automatic file length extension on writes, open a handle with the append flag and use that.
AFIO fatal exits the process if you try to read or write past the current file size, it's a logic error and your program is inherently broken.
This is unacceptable behavior IMO. I would expect an exception in such case.
AFIO assumes the only reason you are reading and writing past the maximum file size is either a logic error or memory corruption. It cannot tell which is which quickly, so we assume it's memory corruption as your program should never have this logic error. And as I explained before, if AFIO thinks memory corruption has occurred it can no longer know if what it has been told to do is correct or wrong, and therefore not terminating the process could cause damage to other people's data.
I do have a very big concern that it is a denial of service attack on anything using AFIO by simply shrinking a file it is using from another process to cause the AFIO using process to fatal exit, so I will be trying to improve on the current situation in the engine rewrite. It's not as easy as it looks if you want to keep reads and writes fast, believe me, but again the ASIO reactor is in my way because that is code I can't control. Once I've written a replacement reactor I am hoping it can track scatter-gather writes a lot better than ASIO can and therefore handle this situation better without adding much overhead for tracking the individual operations.
Given that you go to great lengths to provide reasonable behavior in the presence of other types of concurrent modification, it is very surprising that the library makes it impossible to handle concurrent file truncation. Writes could also fail due to running out of disk space or quota. Without this AFIO is essentially only usable in highly controlled circumstances, which would seem to make all of the other race guarantees unnecessary. For instance it would not be usable for writing a general file synchronization, backup, or file server program.
On 1 Sep 2015 at 17:40, Jeremy Maitin-Shepard wrote:
Before people go "aha!", remember that afio::handle has proper error handling and has to configure a number of other items, including asking the OS for what its "real" path is and other housekeeping.
Why does the library do that when not asked? Why not collect this data only when the user requests?
The problem is the race free filesystem support which isn't as simple, unfortunately, as simply using the POSIX *at() functions.
You need a minimum of the true canonical path of the fd, and its stat_t struct (specifically the inode, created and modified timestamps). That's at least two additional syscalls just for those, and AFIO may do as many as seven syscalls per handle open depending.
Can you describe in more detail exactly why this information is needed/how it is used?
This is a complex and lengthy answer if answered in full, and is a large part of the AFIO value add. The housekeeping done during handle open is used throughout AFIO to skip work in later operations under the assumption that handle opens are not common. Limiting the discussion to just the problem of race free filesystem on POSIX, imagine the problem of opening the sibling of a file in a directory whose path is constantly changing. POSIX does not provide an API for opening the parent directory of an open file descriptor. To work around this, you must first get the canonical current path of the open file descriptor, strip off the end, open that directory and then use that as a base directory for opening a file with the same leafname as your original file. You then loop all that if you fail to open that leafname, or the leafname opened has a different inode. Once you have the correct parent directory, you can open the sibling. This is an example of where caching the stat_t of handle during open saves syscalls and branches in more performance important APIs later on. A similar problem exists for race free file deletions and a long list of other scenarios. The cause is a number of defects in the POSIX race free APIs. The Austin Working Group are aware of the problem. Windows doesn't have problems with race free siblings and deletions due to a much better thought through race free API, but it does have other problems with deletions not being actual deletions and different workarounds are needed there.
You can disable the race free semantics, which are slow, using afio::file_flags::no_race_protection. However AFIO fundamentally assumes that any file system path consuming function is slow and will not be frequently called by applications expecting high performance, so work is deliberately loaded into path consuming functions to keep it away from all other functions.
If you have a need to do a lot of file opens and closes and don't care about races on the file system, you are better off not using AFIO. STL iostreams is perfectly good for this as the only OSs which allow async file opens and closes is QNX and the Hurd.
There is certainly a large gap between what is possible with STL iostreams or Boost filesystem and what is possible using platform-specific APIs for file system manipulations. While it is obviously your choice what scope to pick, I think it may be possible for it to be usable for everything you intend to do with it, e.g. for writing a database backend, but also to be much more widely useful.
It always easy to say "it should do everything optimally". If I had more resources I could do much better than I intend to do. As it stands, without sponsorship you get just 350-400 hours per year, that's just two months full time equivalent. It's very limiting. You have to rationalise.
Stability with respect to what? Do you expect native handles to be somehow different on the same platform?
AFIO's design allows many backends e.g. ZIP archives, HTTP etc. So yes, native handles can vary on the same platform and I needed to choose one for the ABI.
Perhaps you could provide a less generic API for the native platform filesystem, and then once you also have support for ZIP archives, etc. create a more generic interface on top. While archive files and different network protocols like WebDAV and FTP can certainly be presented as file systems, particularly for read-only access, and indeed it is often possible to access them as file systems through the native platform APIs, they tend to differ sufficiently from file native systems that a different API may be more suitable.
I think this is another storm in a teacup. Dropping generic filesystem backends just because native_handle() doesn't return an int? Seems way overkill. Generic filesystem backends could let you do loopback mounts and a long list of value add scenarios. I would consider them a vital design point.
std::vector does not copy memory on C++ 11 when returned by value.
Correction: when returned with a move constructor or RVO.
Which AFIO makes sure of, and I have verified as working.
What I meant is that returning std::vector
means that you have to allocate memory for each path. In contrast, with a callback-based API, you could reuse the same directory_entry for all of the entries.
I can see what you mean. Ok, I'll have a think about it during the engine refactor.
I do have a very big concern that it is a denial of service attack on anything using AFIO by simply shrinking a file it is using from another process to cause the AFIO using process to fatal exit, so I will be trying to improve on the current situation in the engine rewrite. It's not as easy as it looks if you want to keep reads and writes fast, believe me, but again the ASIO reactor is in my way because that is code I can't control. Once I've written a replacement reactor I am hoping it can track scatter-gather writes a lot better than ASIO can and therefore handle this situation better without adding much overhead for tracking the individual operations.
Given that you go to great lengths to provide reasonable behavior in the presence of other types of concurrent modification, it is very surprising that the library makes it impossible to handle concurrent file truncation.
It's more we explicitly give up on trying. File length queries are inherently useless. NTFS and ZFS have a particular knack of straight out lying to you for some random length period. I eventually decided it was safer to just not try.
Writes could also fail due to running out of disk space or quota. Without this AFIO is essentially only usable in highly controlled circumstances, which would seem to make all of the other race guarantees unnecessary. For instance it would not be usable for writing a general file synchronization, backup, or file server program.
Also correct. Delayed allocation means the file system may only try to actually allocate storage on a first write into that extent, and that might fail. This would then blow up with a fatal app exit thanks to AFIO's current implementation. As I said, I am very aware of this, I just needed lightweight futures done before I could start the ASIO reactor replacement. In the meantime, nobody attempting database type systems ever allows the filing system to run out of free space. It's still useful for those sorts of application. Most major operating systems still can suffer permanent unfixable damage from running out of free space on their system drive, even in this modern age. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Tue, Sep 1, 2015 at 7:36 PM, Niall Douglas
This is a complex and lengthy answer if answered in full, and is a large part of the AFIO value add. The housekeeping done during handle open is used throughout AFIO to skip work in later operations under the assumption that handle opens are not common.
Limiting the discussion to just the problem of race free filesystem on POSIX, imagine the problem of opening the sibling of a file in a directory whose path is constantly changing. POSIX does not provide an API for opening the parent directory of an open file descriptor. To work around this, you must first get the canonical current path of the open file descriptor, strip off the end, open that directory and then use that as a base directory for opening a file with the same leafname as your original file. You then loop all that if you fail to open that leafname, or the leafname opened has a different inode. Once you have the correct parent directory, you can open the sibling. This is an example of where caching the stat_t of handle during open saves syscalls and branches in more performance important APIs later on.
The normal way to do this with POSIX *at APIs would be to just open a handle to the directory in the first place. I suppose the purpose of this more complex approach is to avoid having to keep an extra file descriptor to the directory open, or to allow the user to open a sibling file from an arbitrary AFIO file handle without preparing in advance (as I suppose would be required by your shadow file-based locking approach). It does seem like rather specific and not necessarily all that common functionality to require users to pay for by default.
A similar problem exists for race free file deletions and a long list of other scenarios. The cause is a number of defects in the POSIX race free APIs. The Austin Working Group are aware of the problem. Windows doesn't have problems with race free siblings and deletions due to a much better thought through race free API, but it does have other problems with deletions not being actual deletions and different workarounds are needed there.
Personally, I would prefer an API that lets me pay only for what I need. You could expose the low-level platform-specific behavior but also provide higher-level operations that have added complexity to avoid races or emulate behavior not provided natively.
You can disable the race free semantics, which are slow, using
afio::file_flags::no_race_protection. However AFIO fundamentally assumes that any file system path consuming function is slow and will not be frequently called by applications expecting high performance, so work is deliberately loaded into path consuming functions to keep it away from all other functions.
If you have a need to do a lot of file opens and closes and don't care about races on the file system, you are better off not using AFIO. STL iostreams is perfectly good for this as the only OSs which allow async file opens and closes is QNX and the Hurd.
There is certainly a large gap between what is possible with STL iostreams or Boost filesystem and what is possible using platform-specific APIs for file system manipulations. While it is obviously your choice what scope to pick, I think it may be possible for it to be usable for everything you intend to do with it, e.g. for writing a database backend, but also to be much more widely useful.
It always easy to say "it should do everything optimally". If I had more resources I could do much better than I intend to do. As it stands, without sponsorship you get just 350-400 hours per year, that's just two months full time equivalent. It's very limiting. You have to rationalise.
I certainly understand. I'm just trying to convey what I"d like to see in a C++ filesystem API.
Stability with respect to what? Do you expect native handles to be somehow different on the same platform?
AFIO's design allows many backends e.g. ZIP archives, HTTP etc. So yes, native handles can vary on the same platform and I needed to choose one for the ABI.
Perhaps you could provide a less generic API for the native platform filesystem, and then once you also have support for ZIP archives, etc. create a more generic interface on top. While archive files and different network protocols like WebDAV and FTP can certainly be presented as file systems, particularly for read-only access, and indeed it is often possible to access them as file systems through the native platform APIs, they tend to differ sufficiently from file native systems that a different API may be more suitable.
I think this is another storm in a teacup. Dropping generic filesystem backends just because native_handle() doesn't return an int? Seems way overkill.
Generic filesystem backends could let you do loopback mounts and a long list of value add scenarios. I would consider them a vital design point.
I suspect that there may be a relatively easy solution to the particular problem of type safety for native_handle(), e.g. by exposing a platform-specific handle type, but also having some type erasure. I agree that having a generic interface to filesystem-like things is potentially useful. However, I think this is quite a tricky thing, and it is difficult to define such an interface without looking at the capabilities of all of the backends you'd like to support and what functionality different applications would like to use. For instance, some backends might only support reading/writing entire files at once. Some might not allow random access. Some might only allow appending new files. Symlinks, hardlinks, file permissions, modes, flags and other metadata may not be supported or may operate completely differently. (Even if e.g. POSIX file owner/group and mode are supported, the semantics will in general be completely different than for a local filesystem.) Using Boost.Filesystem path doesn't really make sense for anything other than the native OS filesystem APIs. Indeed there are numerous existing libraries/systems that try to do this, and even ways to expose arbitrary things as filesystems to the operating system, e.g. FUSE on Linux, so that individual programs don't need to concern themselves with it. However, there tend to be drawbacks to trying to make these pseudo-filesystems appear as regular filesystems.
Also correct. Delayed allocation means the file system may only try to actually allocate storage on a first write into that extent, and that might fail. This would then blow up with a fatal app exit thanks to AFIO's current implementation. As I said, I am very aware of this, I just needed lightweight futures done before I could start the ASIO reactor replacement.
It isn't clear to me why it would be particularly hard to expose this as a regular error rather than a fatal one, but it sounds like you are planning to fix it anyway.
On 1 Sep 2015 at 23:51, Jeremy Maitin-Shepard wrote:
Limiting the discussion to just the problem of race free filesystem on POSIX, imagine the problem of opening the sibling of a file in a directory whose path is constantly changing. POSIX does not provide an API for opening the parent directory of an open file descriptor. To work around this, you must first get the canonical current path of the open file descriptor, strip off the end, open that directory and then use that as a base directory for opening a file with the same leafname as your original file. You then loop all that if you fail to open that leafname, or the leafname opened has a different inode. Once you have the correct parent directory, you can open the sibling. This is an example of where caching the stat_t of handle during open saves syscalls and branches in more performance important APIs later on.
The normal way to do this with POSIX *at APIs would be to just open a handle to the directory in the first place. I suppose the purpose of this more complex approach is to avoid having to keep an extra file descriptor to the directory open, or to allow the user to open a sibling file from an arbitrary AFIO file handle without preparing in advance (as I suppose would be required by your shadow file-based locking approach). It does seem like rather specific and not necessarily all that common functionality to require users to pay for by default.
The approach you just suggested doesn't handle the case where you open a file, and then it gets moved before the sibling lookup. AFIO can be asked actually to cache the containing directory of a handle. It will attempt to use that handle, when available, to skip inode looping. This feature needs to be specifically enabled as it is off by default.
A similar problem exists for race free file deletions and a long list of other scenarios. The cause is a number of defects in the POSIX race free APIs. The Austin Working Group are aware of the problem. Windows doesn't have problems with race free siblings and deletions due to a much better thought through race free API, but it does have other problems with deletions not being actual deletions and different workarounds are needed there.
Personally, I would prefer an API that lets me pay only for what I need. You could expose the low-level platform-specific behavior but also provide higher-level operations that have added complexity to avoid races or emulate behavior not provided natively.
As you're almost certainly aware by now, I benchmark each feature and decide if it's worth defaulting to on or defaulting to off given my best judgement of all the factors involved. If you really want race protection off all the time, simply instantiate a dispatcher with a file_flags mask which disables race protection for all operations on that dispatcher. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Wed, Sep 2, 2015 at 11:50 AM, Niall Douglas
On 1 Sep 2015 at 23:51, Jeremy Maitin-Shepard wrote:
The normal way to do this with POSIX *at APIs would be to just open a handle to the directory in the first place. I suppose the purpose of this more complex approach is to avoid having to keep an extra file descriptor to the directory open, or to allow the user to open a sibling file from an arbitrary AFIO file handle without preparing in advance (as I suppose would be required by your shadow file-based locking approach). It does seem like rather specific and not necessarily all that common functionality to require users to pay for by default.
The approach you just suggested doesn't handle the case where you open a file, and then it gets moved before the sibling lookup.
Fair enough; I think the desired semantics would depend on the particular case. It seems there is always a time at which you issue the open call in order to open/create the sibling file, and there will inevitably be a race between that and concurrently moving the original file.
On 2 Sep 2015 at 15:44, Jeremy Maitin-Shepard wrote:
The approach you just suggested doesn't handle the case where you open a file, and then it gets moved before the sibling lookup.
Fair enough; I think the desired semantics would depend on the particular case. It seems there is always a time at which you issue the open call in order to open/create the sibling file, and there will inevitably be a race between that and concurrently moving the original file.
Indeed. AFIO generally provides race guarantees for sibling lookups to the level of the parent directory only i.e. the parent directory and anything above it may be permuting and that is not racy, but the contents of the parent directory and deeper are not race free. Each operation has different guarantees on different platforms, and those are listed per operation in the reference docs. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Can you describe in more detail exactly why this information is needed/how it is used?
This is a complex and lengthy answer if answered in full, and is a large part of the AFIO value add. The housekeeping done during handle open is used throughout AFIO to skip work in later operations under the assumption that handle opens are not common.
Limiting the discussion to just the problem of race free filesystem on POSIX, imagine the problem of opening the sibling of a file in a directory whose path is constantly changing. POSIX does not provide an API for opening the parent directory of an open file descriptor. To work around this, you must first get the canonical current path of the open file descriptor, strip off the end, open that directory and then use that as a base directory for opening a file with the same leafname as your original file. You then loop all that if you fail to open that leafname, or the leafname opened has a different inode. Once you have the correct parent directory, you can open the sibling. This is an example of where caching the stat_t of handle during open saves syscalls and branches in more performance important APIs later on.
Sorry for asking a question again (and I'm sure Niall will ignore it because he judges it as 'disrespectful, petty, or juvenile', but perhaps somebody else can answer this): In another mail you said AFIO wouldn't cache anything, thus must do a lot of syscalls over and over again, which is the reason for it to be slow. How does that relate to what you said above? Sorry if I misunderstood something - again. Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu
On 2 Sep 2015 at 7:20, Hartmut Kaiser wrote:
Limiting the discussion to just the problem of race free filesystem on POSIX, imagine the problem of opening the sibling of a file in a directory whose path is constantly changing. POSIX does not provide an API for opening the parent directory of an open file descriptor. To work around this, you must first get the canonical current path of the open file descriptor, strip off the end, open that directory and then use that as a base directory for opening a file with the same leafname as your original file. You then loop all that if you fail to open that leafname, or the leafname opened has a different inode. Once you have the correct parent directory, you can open the sibling. This is an example of where caching the stat_t of handle during open saves syscalls and branches in more performance important APIs later on.
Sorry for asking a question again (and I'm sure Niall will ignore it because he judges it as 'disrespectful, petty, or juvenile', but perhaps somebody else can answer this):
You could treat me with the respect due to someone working in this field for several years the same as the respect I treat you for your ample experience in your domain. If I treated you the way you treat me, you would explode as we have seen before with you shouting at other people on this list who you felt were being disrespectful to you. I am not incompetent. If I chose a design, it was for a reason. It may have been a suboptimal choice, but it is not due to lack of experience nor a lack of prior research. I have been programming for over twenty years, and more than fifteen commercially. I have a very different style and approach to you, but that does not make it inferior or to be mocked or attacked just because I am imprecise in your opinion.
In another mail you said AFIO wouldn't cache anything, thus must do a lot of syscalls over and over again, which is the reason for it to be slow.
We don't second guess the OS. If the OS lies to us - and it does, frequently - we pass through those lies exactly. Similarly, if you loop requesting the length of a file, you'll get back exactly what the OS tells us each time. We always ask the OS for the length if you ask AFIO for the length. No caching. However the semantics emulation and workaround code still needs to correctly emulate any missing semantics on some platform in order to provide a universal file system model to the AFIO user. AFIO provides race-free sibling lookup which works just fine on Windows, but requires some extra work on POSIX, so on POSIX it runs the algorithm described above. The difference is not unlike the difference between FreeBSD and Linux with respect to POSIX. POSIX defines a universal portable standard, and both Linux and FreeBSD implement most of that standard. FreeBSD implements POSIX far more accurately than Linux, and has correspondingly lower peak performance in everything from mutexs to opening files. However, once one includes the cost of the workarounds for Linux's divergences from POSIX in application code, much of that performance gap is eliminated. FreeBSD also scales out much better than Linux because the lower peak performance in a 1-4 CPU machine is really a situation with a higher fixed cost but a less steep slope on the variable cost curve, so as you ramp up the hardware FreeBSD scales better. AFIO is similar to POSIX here. It provides a universal standard model. All platforms require workarounds to match that model, but the workarounds vary between even Linux and FreeBSD. If AFIO must execute extra syscalls on some particular platform in some particular case, you should accept that as a sunk cost on that platform and not worth worrying about in the bigger picture as the simplified application code with fewer workarounds is likely to perform better, be easier to maintain, and transparently improves as OS facilities improve. Your application code is also much more portable. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Limiting the discussion to just the problem of race free filesystem on POSIX, imagine the problem of opening the sibling of a file in a directory whose path is constantly changing. POSIX does not provide an API for opening the parent directory of an open file descriptor. To work around this, you must first get the canonical current path of the open file descriptor, strip off the end, open that directory and then use that as a base directory for opening a file with the same leafname as your original file. You then loop all that if you fail to open that leafname, or the leafname opened has a different inode. Once you have the correct parent directory, you can open the sibling. This is an example of where caching the stat_t of handle during open saves syscalls and branches in more performance important APIs later on.
Sorry for asking a question again (and I'm sure Niall will ignore it because he judges it as 'disrespectful, petty, or juvenile', but perhaps somebody else can answer this):
You could treat me with the respect due to someone working in this field for several years the same as the respect I treat you for your ample experience in your domain. If I treated you the way you treat me, you would explode as we have seen before with you shouting at other people on this list who you felt were being disrespectful to you.
I am not incompetent. If I chose a design, it was for a reason. It may have been a suboptimal choice, but it is not due to lack of experience nor a lack of prior research. I have been programming for over twenty years, and more than fifteen commercially. I have a very different style and approach to you, but that does not make it inferior or to be mocked or attacked just because I am imprecise in your opinion.
In another mail you said AFIO wouldn't cache anything, thus must do a lot of syscalls over and over again, which is the reason for it to be slow.
We don't second guess the OS. If the OS lies to us - and it does, frequently - we pass through those lies exactly. Similarly, if you loop requesting the length of a file, you'll get back exactly what the OS tells us each time. We always ask the OS for the length if you ask AFIO for the length. No caching.
However the semantics emulation and workaround code still needs to correctly emulate any missing semantics on some platform in order to provide a universal file system model to the AFIO user. AFIO provides race-free sibling lookup which works just fine on Windows, but requires some extra work on POSIX, so on POSIX it runs the algorithm described above.
The difference is not unlike the difference between FreeBSD and Linux with respect to POSIX. POSIX defines a universal portable standard, and both Linux and FreeBSD implement most of that standard. FreeBSD implements POSIX far more accurately than Linux, and has correspondingly lower peak performance in everything from mutexs to opening files.
However, once one includes the cost of the workarounds for Linux's divergences from POSIX in application code, much of that performance gap is eliminated. FreeBSD also scales out much better than Linux because the lower peak performance in a 1-4 CPU machine is really a situation with a higher fixed cost but a less steep slope on the variable cost curve, so as you ramp up the hardware FreeBSD scales better.
AFIO is similar to POSIX here. It provides a universal standard model. All platforms require workarounds to match that model, but the workarounds vary between even Linux and FreeBSD. If AFIO must execute extra syscalls on some particular platform in some particular case, you should accept that as a sunk cost on that platform and not worth worrying about in the bigger picture as the simplified application code with fewer workarounds is likely to perform better, be easier to maintain, and transparently improves as OS facilities improve. Your application code is also much more portable.
Thanks! Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu
Niall Douglas wrote
On 31 Aug 2015 at 20:39, Andrey Semashev wrote: Please do correct me if I am wrong, but I had thought that this is defined behaviour:
int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a);
This is certainly a very common pattern in C.
Although it's a common pattern, it's not correct. It has UB since the standard doesn't guarantee that sizeof(void*) == sizeof(int). The issues arise in 64 bit systems where memory addresses are 64 bits wide and int is 32 bits wide. The safe way to handle integer conversion of pointers is through std::intptr_t, which only purpose is to give an integer type guaranteed to have same width as ptrs. -- View this message in context: http://boost.2283326.n4.nabble.com/AFIO-Formal-review-tp4679535p4679636.html Sent from the Boost - Dev mailing list archive at Nabble.com.
On 9/2/2015 4:19 AM, Manu343726 wrote:
Niall Douglas wrote
On 31 Aug 2015 at 20:39, Andrey Semashev wrote: Please do correct me if I am wrong, but I had thought that this is defined behaviour:
int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a);
This is certainly a very common pattern in C.
Although it's a common pattern, it's not correct. It has UB since the standard doesn't guarantee that sizeof(void*) == sizeof(int). The issues arise in 64 bit systems where memory addresses are 64 bits wide and int is 32 bits wide. The safe way to handle integer conversion of pointers is through std::intptr_t, which only purpose is to give an integer type guaranteed to have same width as ptrs.
I don't see how a 64bit pointer can't store a 32bit integer. He's not converting some arbitrary void* to an int in the third line. He knows it is actually a 32bit int stored in a void*.
On 9/12/2015 3:39 PM, Michael Marcin wrote:
I don't see how a 64bit pointer can't store a 32bit integer. He's not converting some arbitrary void* to an int in the third line. He knows it is actually a 32bit int stored in a void*.
Oops my newsreader wasn't updating, didn't realize this was old and already discussed. Apologies for the noise.
On 2015-09-01 18:42, Niall Douglas wrote:
Please do correct me if I am wrong, but I had thought that this is defined behaviour:
int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a);
This is certainly a very common pattern in C.
I believe this is implementation-defined. Based on N3797 [expr.reinterpret.cast], paragraph 5: "A value of integral type or enumeration type can be explicitly converted to a pointer. A pointer converted to an integer of sufficient size (if any such exists on the implementation) and back to the same pointer type will have its original value; mappings between pointers and integers are otherwise implementation-defined." The behaviour that is defined (pointer to integer and back) is the reverse of the one you want. John Bytheway
On 2 Sep 2015 at 5:31, John Bytheway wrote:
Please do correct me if I am wrong, but I had thought that this is defined behaviour:
int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a);
This is certainly a very common pattern in C.
I believe this is implementation-defined. Based on N3797 [expr.reinterpret.cast], paragraph 5:
"A value of integral type or enumeration type can be explicitly converted to a pointer. A pointer converted to an integer of sufficient size (if any such exists on the implementation) and back to the same pointer type will have its original value; mappings between pointers and integers are otherwise implementation-defined."
The behaviour that is defined (pointer to integer and back) is the reverse of the one you want.
That's the whole point of the intermediate size_t cast. int to size_t is defined behaviour, size_t to void * is defined behaviour, void * to size_t is defined behaviour, size_t to int is defined behaviour. Older code, and still a lot of C code, directly casts between int and void * and back again, even though strictly speaking that's a no no. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 02.09.2015 20:55, Niall Douglas wrote:
That's the whole point of the intermediate size_t cast. int to size_t is defined behaviour, size_t to void * is defined behaviour, void * to size_t is defined behaviour, size_t to int is defined behaviour.
I am absolutely no expert in C++, but I was under the impression that while it's highly likely, it's by no means guaranteed that size_t can hold a pointer. IIRC if a platform has segmented memory then this assumption could break. In any case, uintptr_t should be available on your target compilers no? If so, that sounds like the better choice to me. I may be showing my inexperience, so feel free to ignore the above if it's bonkers :) That said, as a humble Boost user, native_handle() returning void* sounds a bit iffy to me. Why is it a pointer if it does not point to anything but should rather be casted to the appropriate type by the user? Cheers - Asbjørn
On 2 Sep 2015 at 23:20, Asbjørn wrote:
That's the whole point of the intermediate size_t cast. int to size_t is defined behaviour, size_t to void * is defined behaviour, void * to size_t is defined behaviour, size_t to int is defined behaviour.
I am absolutely no expert in C++, but I was under the impression that while it's highly likely, it's by no means guaranteed that size_t can hold a pointer.
True. It is however true on all the platforms AFIO supports. That is still no excuse however. size_t is for array indexes, not addresses where a uintptr_t is a better type.
IIRC if a platform has segmented memory then this assumption could break.
In any case, uintptr_t should be available on your target compilers no? If so, that sounds like the better choice to me.
I've logged the issue to https://github.com/BoostGSoC13/boost.afio/issues/117 to do just that.
That said, as a humble Boost user, native_handle() returning void* sounds a bit iffy to me. Why is it a pointer if it does not point to anything but should rather be casted to the appropriate type by the user?
Old habits of a C programmer: void * is for any "carry maximum information". Besides, there is form: Boost.Python defines the standard opaque Python-can't-understand-this type as a void * (I contributed the implementation myself a very long time ago). Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 03.09.2015 01:21, Niall Douglas wrote:
On 2 Sep 2015 at 23:20, Asbjørn wrote:
That said, as a humble Boost user, native_handle() returning void* sounds a bit iffy to me. Why is it a pointer if it does not point to anything but should rather be casted to the appropriate type by the user?
Old habits of a C programmer: void * is for any "carry maximum information". Besides, there is form: Boost.Python defines the standard opaque Python-can't-understand-this type as a void * (I contributed the implementation myself a very long time ago).
Yeah I can understand that. However personally, again as a humble Boost user, I would prefer if you "dress it up" by a typedef so that in the docs and user code there is no mention of this being a void*. This would be a bit similar to ASIO's "native_handle_type". Cheers - Asbjørn
On 02.09.2015 21:55, Niall Douglas wrote:
On 2 Sep 2015 at 5:31, John Bytheway wrote:
Please do correct me if I am wrong, but I had thought that this is defined behaviour:
int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a);
This is certainly a very common pattern in C.
I believe this is implementation-defined. Based on N3797 [expr.reinterpret.cast], paragraph 5:
"A value of integral type or enumeration type can be explicitly converted to a pointer. A pointer converted to an integer of sufficient size (if any such exists on the implementation) and back to the same pointer type will have its original value; mappings between pointers and integers are otherwise implementation-defined."
The behaviour that is defined (pointer to integer and back) is the reverse of the one you want.
That's the whole point of the intermediate size_t cast. int to size_t is defined behaviour, size_t to void * is defined behaviour, void * to size_t is defined behaviour, size_t to int is defined behaviour.
Only that the original and resulting ints are equal is not guaranteed.
Older code, and still a lot of C code, directly casts between int and void * and back again, even though strictly speaking that's a no no.
Horrible code exists. No need to add yet more.
On 2015-09-02 14:55, Niall Douglas wrote:
On 2 Sep 2015 at 5:31, John Bytheway wrote:
Please do correct me if I am wrong, but I had thought that this is defined behaviour:
int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a);
This is certainly a very common pattern in C.
I believe this is implementation-defined. Based on N3797 [expr.reinterpret.cast], paragraph 5:
"A value of integral type or enumeration type can be explicitly converted to a pointer. A pointer converted to an integer of sufficient size (if any such exists on the implementation) and back to the same pointer type will have its original value; mappings between pointers and integers are otherwise implementation-defined."
The behaviour that is defined (pointer to integer and back) is the reverse of the one you want.
That's the whole point of the intermediate size_t cast. int to size_t is defined behaviour, size_t to void * is defined behaviour, void * to size_t is defined behaviour, size_t to int is defined behaviour.
To be clear, do you mean standard-defined (as opposed to implementation-defined)? I'm assuming so. Then, as Andrey pointed out, the result is not defined. Only integer values that were obtained from a previous cast from a pointer can be safely cast to a pointer type (unless you have evidence from elsewhere in the standard to suggest more). If you actually want defined behaviour here, I think you should use a union { int; void*; }. That ought not to have any performance penalty. That said, I imagine that in practice the implementation-defined behaviour of what you're doing will be what you want in any implementation where using AFIO makes sense. John
Older code, and still a lot of C code, directly casts between int and void * and back again, even though strictly speaking that's a no no.
Niall
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On 2015-09-04 05:52, John Bytheway wrote:
On 2015-09-02 14:55, Niall Douglas wrote:
On 2 Sep 2015 at 5:31, John Bytheway wrote:
Please do correct me if I am wrong, but I had thought that this is defined behaviour:
int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a);
This is certainly a very common pattern in C.
I believe this is implementation-defined. Based on N3797 [expr.reinterpret.cast], paragraph 5:
"A value of integral type or enumeration type can be explicitly converted to a pointer. A pointer converted to an integer of sufficient size (if any such exists on the implementation) and back to the same pointer type will have its original value; mappings between pointers and integers are otherwise implementation-defined."
The behaviour that is defined (pointer to integer and back) is the reverse of the one you want.
That's the whole point of the intermediate size_t cast. int to size_t is defined behaviour, size_t to void * is defined behaviour, void * to size_t is defined behaviour, size_t to int is defined behaviour.
To be clear, do you mean standard-defined (as opposed to implementation-defined)? I'm assuming so.
Then, as Andrey pointed out, the result is not defined. Only integer values that were obtained from a previous cast from a pointer can be safely cast to a pointer type (unless you have evidence from elsewhere in the standard to suggest more).
If you actually want defined behaviour here, I think you should use a union { int; void*; }. That ought not to have any performance penalty.
I realised that this comment was ambiguous. To clarify: I do *not* mean using a union for type-punning to convert the int to a void*; rather I mean using a union as the native handle type so you never need to convert the int to a void*. John
That said, I imagine that in practice the implementation-defined behaviour of what you're doing will be what you want in any implementation where using AFIO makes sense.
John
Older code, and still a lot of C code, directly casts between int and void * and back again, even though strictly speaking that's a no no.
Niall
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On 02.09.2015 01:42, Niall Douglas wrote:
On 31 Aug 2015 at 20:39, Andrey Semashev wrote:
Before people go "aha!", remember that afio::handle has proper error handling and has to configure a number of other items, including asking the OS for what its "real" path is and other housekeeping.
Why does the library do that when not asked? Why not collect this data only when the user requests?
The problem is the race free filesystem support which isn't as simple, unfortunately, as simply using the POSIX *at() functions.
You need a minimum of the true canonical path of the fd, and its stat_t struct (specifically the inode, created and modified timestamps). That's at least two additional syscalls just for those, and AFIO may do as many as seven syscalls per handle open depending.
But if my program doesn't need these properties, why do I have to pay to obtain them from the system? The argument that opening a file is already expensive doesn't work as you're just making it yet more expensive for no benefit for the user. Also, I don't think I understand how obtaining this additional data helps to achieve race free semantics. But that's probably because I'm not an expert in the field.
You can disable the race free semantics, which are slow, using afio::file_flags::no_race_protection.
Will this avoid obtaining any unnecessary data? I.e. will opening a file then be equivalent to a single syscall, which is to open the file? If so, I think this should be the default and any additional data to obtain should be an opt-in.
If you have a need to do a lot of file opens and closes and don't care about races on the file system, you are better off not using AFIO. STL iostreams is perfectly good for this as the only OSs which allow async file opens and closes is QNX and the Hurd.
It's all about the total total effect of the optimization. I could use std::iostreams to process a 1000 files in serial, or I could spawn 1000 threads and do the same somewhat faster, or I could use Boost.AFIO, hoping to save some development time and avoid CPU overcommit for roughly the same amount of benefit in terms of performance as my naive 1000-thread code. IMO, Boost.AFIO should live up to this expectation, otherwise I don't see much use for it and it should probably not be called AFIO. I think you target two very different goals - provide an async API for file system operations and provide a race-free API to file system. One of the key benefits of making things async is to make it more responsive, if not plain faster. If that is not achieved then there is no point in being async as it just complicates the code. I think many reviewers who took a look at Boost.AFIO were looking for this kind of benefit and seeing you saying that Boost.AFIO should not be used for that is surprising and discouraging. (This is why I questioned the name AFIO above.) Race-free API may be interesting on its own, but apparently being race-free hampers performance, which is essential for the first goal. Additionally, race-free does not mean async, so the race-free filesystem library could be synchronous and as a consequence - simpler to implement and use. Combining these two goals in one library you're making compromises that result in neither a good async library nor a simple race-free one. Perhaps, you should decide what you want to achieve in your library.
- I'd much prefer native_handle() to return the actual appropriate native handle type for the platform, e.g. int on POSIX, rather than a void *. I'm guessing this is done in order to support alternative dispatchers, that aren't necessarily based on OS files at all, in the future, but perhaps a better way of doing this that doesn't require casting the void* to an int, can be found.
It's more ABI stability.
Stability with respect to what? Do you expect native handles to be somehow different on the same platform?
AFIO's design allows many backends e.g. ZIP archives, HTTP etc. So yes, native handles can vary on the same platform and I needed to choose one for the ABI.
I wouldn't call that 'native' handles. In my understanding native means the underlying OS primitive. If you intend to emulate a file system API on top of non-file system entities, such as archives and remote services, then you probably need to design the library very differently. Something closer to ASIO comes to mind - each backend should define its own set of types, including the underlying primitives, and those types should be propagated up to the user's interface. The whole library should be much more flexible and concept-oriented.
Casting void * to int is fine.
No, it's not. It's a hack that you impose on the library users. According to [expr.reinterpret.cast]/4 it's not even required to compile, as int may not be large enough to hold a pointer.
The native handle on POSIX is stored internally as an int. In native_handle(), it is cast to a size_t, and then to a void *.
Please do correct me if I am wrong, but I had thought that this is defined behaviour:
int a=5; void *b=(void *)(size_t) a; int c=(int)(size_t) b; assert(c==a);
This is certainly a very common pattern in C.
The standard defines conversion of a pointer to an integer of sufficient size (which is (u)intptr_t, and not necessarilly size_t) and back and that this roundtrip looses no information. The bit mapping function between pointers and integers is not defined and hence there is no guarantee that any integer can be represented in a pointer. The other way is true though, as long as there exists (u)intptr_t, which is also not guaranteed (i.e. pointers are allowed to be something completely different from integers). Besides all that, requiring your users to do tricks like that is not acceptable, IMO. We're in C++, use the type system it offers.
It's more that pwrite off the end of the file is inherently racy to concurrent read/writers. If you want safe automatic file length extension on writes, open a handle with the append flag and use that.
AFIO fatal exits the process if you try to read or write past the current file size, it's a logic error and your program is inherently broken.
This is unacceptable behavior IMO. I would expect an exception in such case.
AFIO assumes the only reason you are reading and writing past the maximum file size is either a logic error or memory corruption. It cannot tell which is which quickly, so we assume it's memory corruption as your program should never have this logic error. And as I explained before, if AFIO thinks memory corruption has occurred it can no longer know if what it has been told to do is correct or wrong, and therefore not terminating the process could cause damage to other people's data.
I'll repeat myself and say this is being too smart. This situation is perfectly recoverable and does not mean memory corruption (at least, not 100%, by far). It should not result in program termination.
I do have a very big concern that it is a denial of service attack on anything using AFIO by simply shrinking a file it is using from another process to cause the AFIO using process to fatal exit, so I will be trying to improve on the current situation in the engine rewrite. It's not as easy as it looks if you want to keep reads and writes fast, believe me, but again the ASIO reactor is in my way because that is code I can't control. Once I've written a replacement reactor I am hoping it can track scatter-gather writes a lot better than ASIO can and therefore handle this situation better without adding much overhead for tracking the individual operations.
I don't think I understood much of this, but if this makes the library not terminate the process in this situation then go for it.
On 2 Sep 2015 at 14:46, Andrey Semashev wrote:
You can disable the race free semantics, which are slow, using afio::file_flags::no_race_protection.
Will this avoid obtaining any unnecessary data? I.e. will opening a file then be equivalent to a single syscall, which is to open the file? If so, I think this should be the default and any additional data to obtain should be an opt-in.
It definitely breaks race protection, and you lose an undefined amount of that i.e. there is still some race protection, but an undefined quantity. As for less work, it gives no guarantees. Probably yes it will involve many fewer syscalls. The reason it will never be defaulted on is because it introduces substantial behaviour differences between platforms. Even FreeBSD and Linux will no longer behave the same. That's whole point of the emulation and workaround code to fix up platform specific differences so you don't need to think about that when writing code against AFIO. You can program against a standardised, universal filesystem model and not worry about platform specific differences (mostly). If you do disable race protection - and you can turn it off per operation, per handle or per dispatcher - you are explicitly exchanging predictability for an unspecified performance increase.
I think you target two very different goals - provide an async API for file system operations and provide a race-free API to file system. One of the key benefits of making things async is to make it more responsive, if not plain faster.
I will repeat myself once again because you are not listening to me: making things async without a fundamental rearchitecture makes them *slower* not faster. That even goes for socket i/o - feel free to test 1000 threads using a synchronous socket against single threaded ASIO with 1000 sockets. Here are some empirical benchmarks by Google: https://www.mailinator.com/tymaPaulMultithreaded.pdf. Async i/o is *always* slower because you are doing more work. It didn't used to be once upon a time, but in any OS of the past decade it is - as that Google presentation shows. The ONLY good reason to go async is CONTROL. You can achieve better worst case performance with async. You can achieve more easily better design with async. That goes for sockets as well as file i/o.
If that is not achieved then there is no point in being async as it just complicates the code. I think many reviewers who took a look at Boost.AFIO were looking for this kind of benefit and seeing you saying that Boost.AFIO should not be used for that is surprising and discouraging. (This is why I questioned the name AFIO above.)
Many reviewers have inaccurate starting preconceptions. They believe async automatically correlates with faster. That might have been true in the 1990s and early 2000s. It is no longer true, without a fundamental rearchitecture i.e. throw out all your design preconceptions.
Race-free API may be interesting on its own, but apparently being race-free hampers performance, which is essential for the first goal.
Again, I repeat the point about control and worst case behaviour. AFIO can and does produce superb file system performance an order of magnitude or more in excess of anything achievable with the STL or anything in Boost. But you'll need to completely rewrite your algorithms according to file system theory first. If I don't reply to any reply you make, I will have gone on my holidays away from Boost. Thanks though for the feedback Andrey. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 02.09.2015 22:18, Niall Douglas wrote:
On 2 Sep 2015 at 14:46, Andrey Semashev wrote:
The reason it will never be defaulted on is because it introduces substantial behaviour differences between platforms. Even FreeBSD and Linux will no longer behave the same. That's whole point of the emulation and workaround code to fix up platform specific differences so you don't need to think about that when writing code against AFIO. You can program against a standardised, universal filesystem model and not worry about platform specific differences (mostly).
If you do disable race protection - and you can turn it off per operation, per handle or per dispatcher - you are explicitly exchanging predictability for an unspecified performance increase.
Ok, so it's not about just being race-free, it's about providing a portable API. The cost on this I can accept, as long as the portable API is not requiring too much emulation on every platform. I'm not sure this precondition is fulfilled though, based on your comments. I mean, what portable API would require obtaining file metadata and actual path on opening a file?
I think you target two very different goals - provide an async API for file system operations and provide a race-free API to file system. One of the key benefits of making things async is to make it more responsive, if not plain faster.
I will repeat myself once again because you are not listening to me: making things async without a fundamental rearchitecture makes them *slower* not faster. That even goes for socket i/o - feel free to test 1000 threads using a synchronous socket against single threaded ASIO with 1000 sockets.
I wasn't comparing a _single_ threaded async solution to a 1000-thread sync one - that would be silly of me. I would expect AFIO to perform async operations in parallel, involving a thread pool with optimal number of threads, if necessary. I would expect it to be about as fast, and possibly use less memory too. Much depends on the async reactor implementation though.
If that is not achieved then there is no point in being async as it just complicates the code. I think many reviewers who took a look at Boost.AFIO were looking for this kind of benefit and seeing you saying that Boost.AFIO should not be used for that is surprising and discouraging. (This is why I questioned the name AFIO above.)
Many reviewers have inaccurate starting preconceptions. They believe async automatically correlates with faster. That might have been true in the 1990s and early 2000s. It is no longer true, without a fundamental rearchitecture i.e. throw out all your design preconceptions.
Noone's arguing about having to design your code properly to make use of asynchronicity. Async programming is a form of parallel programming, just like multithreaded programming is. Either can be slower or faster than the other for different reasons, but both are arguably faster than an equivalent synchronous single threaded program. That's where the speedup expectation is coming from, and I think it's a fair expectation. Async code is notably harder to write than multithreaded, so you have to give something in return if you design your library the async way. If not performance or resource utilization then what? You mention control and worst case performance, but I can't see why an async program would do any better in this regard than an equivalent multithreaded sync one. Can you give a more concrete example when it does?
Race-free API may be interesting on its own, but apparently being race-free hampers performance, which is essential for the first goal.
Again, I repeat the point about control and worst case behaviour.
AFIO can and does produce superb file system performance an order of magnitude or more in excess of anything achievable with the STL or anything in Boost. But you'll need to completely rewrite your algorithms according to file system theory first.
I'm sorry, you lost me here. You just said that AFIO, being an async library, will always be slower than STL, and now you say its performance is superb?
On 3 Sep 2015 at 1:53, Andrey Semashev wrote:
Race-free API may be interesting on its own, but apparently being race-free hampers performance, which is essential for the first goal.
Again, I repeat the point about control and worst case behaviour.
AFIO can and does produce superb file system performance an order of magnitude or more in excess of anything achievable with the STL or anything in Boost. But you'll need to completely rewrite your algorithms according to file system theory first.
I'm sorry, you lost me here. You just said that AFIO, being an async library, will always be slower than STL, and now you say its performance is superb?
I'm just about to go to bed and we go on family holiday tomorrow before which I'll unsubscribe from boost-dev until January. But just to answer the above, if you program AFIO using the same algorithms and programming model as the STL you will almost always get slower code. AFIO provides much stronger guarantees, and ASIO adds more work. With AFIO you can use algorithms and models much more tightly aligned with how filesystems actually work. Designs not possible with the STL. Such designs can be an order of magnitude faster or better. You can see what I mean in the workshop tutorial where the first example is written using STL iostreams, the second and third using AFIO but also using a file per key-value. The fourth example has its design described in detail and as you will quickly realise, such a design is not possible with the STL or anything currently in Boost. I would expect lookup performance to be at least 10x-1000x faster and write performance at least 10x faster than the STL compatible design of a file per key-value. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Mon, Aug 31, 2015 at 5:18 PM, Niall Douglas
On 31 Aug 2015 at 5:26, Jeremy Maitin-Shepard wrote:
1. Should Boost.AFIO be accepted into Boost? Please state all conditions for acceptance explicity.
In its current state, Boost.AFIO should NOT be accepted into Boost. However, I am confident that Niall will be able to deliver a future version of AFIO that should be accepted into Boost after another review. There is a great need for more powerful and efficient file handling functionality in C++ than is provided by iostreams and Boost.Filesystem/Filesystem TS. I appreciate the enormous complexity of trying to define a multi-platform filesystem interface, and it is clear that Niall has done a ton of very valuable research on this topic.
Thanks.
2. What is your evaluation of the design?
A key strength of the design is that it provides a common interface for asynchronous operations regardless of whether they are internally implemented using Windows IOCP support, async IO on FreeBSD, or regular POSIX operations via a thread pool.
Another key strength is that it exposes handle-based (i.e. *at), rather than path-based APIs where possible in order to minimize race conditions, which tend to be difficult or impossible to avoid completely with file system operations.
I do have a number of concerns about several aspects of the design, however:
Batch API:
- It is not clear what, if any, purpose the batch API serves. The significant added complexity of using it would have to be justified by a significant performance advantage, but it is not at all clear that there is even any performance advantage at all.
- I suggest removing it unless there is a strong argument otherwise.
The batch API may, in the end, not prove to be performance positive for the file:/// URI backend. But I can see it could be of great use for http:// or ftp:// URI backends. Even with a zip:// URI backend, batch write would save a lot of rebuilding the ZIP archive.
File handle API:
- The file handle abstraction in AFIO is very heavy weight, involving a shared_ptr to a structure containing a whole bunch of fields in addition to the native file handle, including a mutex, numerous flags, the path used to open it, a pointer to a handle to the parent directory, a chrono time_point specifying the time the file was opened, the total number of bytes read, written, and written since last fsync using this file handle.
- Because file handles are at the core of AFIO, it is critical that users precisely understand what this abstraction involves, and specifically how it differs from a native file handle.
- I'd be much more comfortable with a much leaner abstraction, like a unique_handle type that just contains the bare native handle, and uses unique ownership semantics. The fat handles impose both computational/memory overhead as well as mental overhead on the programmer. I understand these extra fields are probably necessary to provide various functionality, particularly emulation of behavior provided by other operating systems, but it would be nicer to be able to opt-in to such functionality and only pay for it when needed.
- At the very least, if these fat file handles are kept, a very strong argument needs to be given as a rationale. (Perhaps you have found the overhead to be negligible relative to the cost of the native file handle.)
I really feel the need to strenuously disagree with the notion that AFIO has a "fat" handle. People have no comprehension of the cost of the open()/CreateFile() system call relative to a shared_ptr or any of the other things afio::handle does, and they are fixated on judging AFIO's design choice completely inappropriately as a result.
To show what I mean, here is a gist demonstrating how lightweight AFIO's handle is relatively speaking:
https://gist.github.com/ned14/affa007440f0568e4aa6
This test simply compares opening and closing a file with the same thing plus a shared_ptr creation and destruction.
On Linux: +4.76% On Windows: +1.8%.
Before people go "aha!", remember that afio::handle has proper error handling and has to configure a number of other items, including asking the OS for what its "real" path is and other housekeeping.
When I said that the overhead of shared_ptr was about 0.5%, I meant it. I benchmarked it before taking the decision.
- It should be possible to construct an AFIO handle from a native file descriptor.
That's already in there: simply subclass afio::handle with your own handle type. I can't allow existing afio::handle to use third party descriptors because they need to be configured very exactly on Windows for AFIO to work correctly. This is less of a problem on POSIX, but it still applies there too. When the handle subclass type is third party, AFIO can recognise the situation and handle it correctly.
- On POSIX, AFIO should support supplying platform-specific open flags, like O_NOATIME.
Hmm, maybe. I can see pros and cons. I've logged it to https://github.com/BoostGSoC13/boost.afio/issues/110.
- I'd much prefer native_handle() to return the actual appropriate native handle type for the platform, e.g. int on POSIX, rather than a void *. I'm guessing this is done in order to support alternative dispatchers, that aren't necessarily based on OS files at all, in the future, but perhaps a better way of doing this that doesn't require casting the void* to an int, can be found.
It's more ABI stability. Casting void * to int is fine.
- Given that it is extremely likely for user code to execute from multiple threads given the nature of AFIO, relying on a thread-local current dispatcher seems very error prone.
- I suggest that the thread-local current dispatcher be eliminated, and that the dispatcher always be specified explicitly to any API call that requires one.
Noted.
Directory handle cache:
- A cache of open handles to directories is kept by AFIO by default. This is particularly problematic because (as is documented) it makes reading directory entries thread-unsafe by default. If this is to be kept, a very strong rationale needs to be given, and the behavior must be changed somehow to ensure that directory reading is reliable by default.
The documentation explains exactly why there is this cache, it's a big win despite the complexity it introduces. The problem of races on directory enumeration on shared directory handles is eliminated if you make the number of entries enumerated in a single shot bigger than the total. In practice, this is not a problem, even on 10M item directories, just throw max_items at the problem.
Enumerate API:
- The future-based API of async_enumerate seems to be unnecessarily complicated, due to the need to specify maxitems and deal with making repeated calls, and all of the copying and memory allocation required to return a std::vector of directory_entry.
If you want race free snapshots (up to the platform specific guarantees), simply make sure maxitems far exceeds any potential contents.
std::vector does not copy memory on C++ 11 when returned by value.
I suggest instead something like passing in a std::function
const &next)> callback (possibly replacing expected by the new name for your monad type). An empty next value means end of directory has been reached. Returning true means continue listing the directory, returning false means stop listing the directory. I didn't do this because we actually do fetch maxitems from the kernel in a single shot. If you ask for 10M items, all 10M items are returned in a single kernel syscall as a single, race free snapshot. Calling some callback per item is therefore not useful.
On POSIX, with the current implementation directory enumeration isn't particularly race free past the inode and type, which is still quite useful much of the time. However I had in mind in the future we'd pull the directory inode by hand in a single shot and parse it into the AFIO output. FreeBSD lets you do that without any filing system specific code, unfortunately Linux requires you to understand ext4 inode layouts.
My point is that you need to think not in terms of conventional directory enumeration. AFIO will enumerate a 1M item directory faster than a traditional design can do 10,000 items. It's so fast you can afford to fetch more than you need and throw away the excess.
- Operations including rmdir, rmfile, symlink, rmsymlink (as well as async variants) that normally would not necessarily even operate on file handles, actually return a handle_ptr. Perhaps if on some platforms a handle has to be opened, then there could be a way for the user to specify that a handle is desired. Otherwise, it would make sense to just return void or std::future<void> equivalent for async variants.
Already logged.
- The documentation notes that on MS Windows, DOS short names are not supported. I don't have much familiarity with Windows, but if programs generally accept DOS short names when a filename is required, then AFIO should probably support these names as well.
The cost of this support is high, and the benefit very low unless you are likely to run original 16bit DOS programs on files generated by AFIO (note 64 bit Windows no longer can run 16 bit DOS programs). You'll run into long path problems on Windows long before 8.3 name compatibility problems.
- The documentation notes: "Don't do directory enumerations from an x86 binary on a x64 Windows system. This is due to a bug in Microsoft's WOW64 syscall translation layer which Microsoft have decided is wontfix. " Given that a major purpose of AFIO is to provide a portable abstraction that works around OS limitations, AFIO needs to support directory enumeration even in this case (e.g. by detecting WOW64 and using a synchronous call via a threadpool in that case).
This problem is caused by how ASIO's reactor works and is due to code I can't modify in ASIO. It'll go away when I rewrite ASIO's reactor using lightweight futures because I can control the source code.
- It is noted that to do an asynchronous stat you have to use enumerate with an appropriate glob specified, but constructing such a glob may be non-trivial if the path contains characters that are interpreted specially by the glob syntax, and impossible to do in a race-free manner.
On POSIX fnmatch() which is used to implement glob allows escaping of the metacharacters. On Windows the glob metacharacters are illegal in a path anyway.
- The implementation might be better split up into multiple independent files. There are various independent facilities, like secded_ecc, that could be placed in a separate file.
Already logged.
- Although this tends to be a problem with most Boost libraries, it would be nice if the compile times could be reduced, particularly when not in header-only mode. Some libraries like Boost Spirit have an interface inherently based on template metaprogramming, such that long compile times are pretty much unavoidable, but the Boost AFIO interface seems to be mostly non-template based, which suggests that it may be possible to get pretty good compile times.
AFIO if configured to do so can achieve excellent compile times as it's something I've worked hard to optimise: https://boostgsoc13.github.io/boost.afio/doc/html/afio/FAQ/slow_compil e.html
4. What is your evaluation of the documentation?
- The extensive documentation about race guarantees is extremely useful.
- I appreciate the in-depth examples.
- Much more design rationale is needed.
- The term "filing system" is used in place of the more common "file system", leading me to wonder if there is a difference in meaning.
- Every instance where there is a platform-specific #ifdef in an example should be evaluated, so that you can figure out the appropriate platform-independent abstraction that could be added to AFIO in order to move that #ifdef into the library and out of user code.
- There is an attempt to document differences in behavior on different platforms, but this is not done sufficiently. For example, it is stated that async_write requires that the destination region does not go off the end of the file, but on POSIX pwrite supports this just fine as a way to extend the file. I assume that this restriction applies to Windows only.
It's more that pwrite off the end of the file is inherently racy to concurrent read/writers. If you want safe automatic file length extension on writes, open a handle with the append flag and use that.
AFIO fatal exits the process if you try to read or write past the current file size, it's a logic error and your program is inherently broken.
- The semantics of the Batch API are not precisely documented, nor is any rationale given for it. For instance, I understand that the operations themselves cannot happen atomically, since that is not supported by operating systems, though this is not clearly stated. It is stated: "Once a batch of input ops has been verified at the point of entry as not errored, you are guaranteed that the batch is atomically scheduled as a whole, unless a failure to allocate memory occurs." Does this means that if the *_req object involves futures, it waits for all of them to be ready before launching any of the operations in the batch? Is that the purpose of the Batch API, to do a wait_all on all of the relevant futures, then to launch each operation? This issue goes away though if the Batch API is eliminated, as seems to make sense.
- It would be useful to document for each operation which underlying system calls it invokes on each platform.
- Many operations other than open (e.g. rmfile) take file_flags, but it is not specified which flags are supported.
Good point. Logged to https://github.com/BoostGSoC13/boost.afio/issues/111.
- References to historical versions of AFIO (prior to the review) should be removed from the main parts of the documentation, since they only distract from the useful information. This information could be kept in a different section at the end for the extra curious.
- Formatting of the "description" in the Boost Quickbook format function documentation has some problems: the lack of line breaks between the general description, other notes, and the common note about the use of NT kernel paths make it difficult to read.
5. What is your evaluation of the potential usefulness of the library?
The library is potentially extremely useful. C++ and Boost iostreams and Boost/TS Filesystem provide only very limited functionality. The C library libuv is a close competitor but seems to offer less functionality and a less convenient API. For achieving similar functionality, the status quo in C++ is to rely on platform-specific C APIs that are much more cumbersome to use.
6. Did you try to use the library? With what compiler? Did you have any problems?
I tried some very simple usage of the library with GCC 5.2.0 on Linux. There were a few warnings (variable set but not used) with -Wall that would be best eliminated.
Logged to https://github.com/BoostGSoC13/boost.afio/issues/112
- The error messages are not ideal: for instance, in the case that there was an error opening a file, this is the result:
Dynamic exception type: boost::system::system_error std::exception::what: File '/home/jbms/tmp/afio-test/test.txt' not found [Host OS Error: No such file or directory (2) in '/home/jbms/tmp/boost.afio/include/boost/afio/v2/detail/impl/afio.ipp': async_io_handle_posix:805]: No such file or directory. Op was scheduled at: 1. 0x42fb4f: ./test (+0x2fb4f) : No such file or directory
Some of this is only useful when debugging AFIO, some only useful when debugging the application, and there is a lot of redundancy, for instance the actual error "No such file or directory" is shown in 3 places. Ideally there would be a way to get a simple message along the lines of "Error opening file '/home/jbms/tmp/afio-test/test.txt': No such file or directory" that could be shown directly to the user.
Those "No such file or directory" were from the stack backtracer. I assume it could not figure out your debug info.
Regarding your request, in the lightweight futures rewrite we will exclusively use std::error_code for unexceptional errors, and therefore you will have exactly as you ask.
- A separate issue is that attempting to read a range that is not present in a file, something that the library should handle elegantly, leads to the program aborting immediately with an error message, rather than allowing the error to be handled:
There is no way of gracefully handling this without introducing a lot of complexity to IPC with concurrent readers and writers.
Besides, it's a logic error to do i/o outside the file's maximum permitted extent, and this is enforced in all asynchronous file i/o applications far outside AFIO. The reason I chose a fatal exception is that this is not a recoverable error, this is a fundamental mistake in your implementation (or memory corruption) and you need to fix it.
what happens if another process truncates the file while I'm writing to it? Also is the 'max size' cached? What happens if another process extends the file size, notifies me out of band of the new size and I write beyond the original limit? I would certainly hope that the application won't crash in neither scenario. It must never be possible for a process to cause another process in another address space to crash (discounting super user privileges or ptrace of course). -- gpd -- gpd
On 31 Aug 2015 at 20:31, Giovanni Piero Deretta wrote:
what happens if another process truncates the file while I'm writing to it?
Right now, a denial of service attack. I'm aware of it (see other thread), but I need to replace the ASIO reactor to fix it.
Also is the 'max size' cached? What happens if another process extends the file size, notifies me out of band of the new size and I write beyond the original limit?
AFIO doesn't second guess the kernel, and doesn't cache anything anywhere at all in data structures it manages. There is no out of bounds checking in the scatter gather read/write implementation - all that AFIO knows is a read or write comes back partial because that is all ASIO tells us. As you are guaranteed by the OS that filesystem i/o is NEVER partial, something is seriously wrong, so we fatal exit. Hence the message "buffers not filled".
I would certainly hope that the application won't crash in neither scenario. It must never be possible for a process to cause another process in another address space to crash (discounting super user privileges or ptrace of course).
It better for controlled fatal exit than a security breach. But yes, I agree. This denial of service attack is a problem, and I hope to fix it in the engine refactor once ASIO isn't in the way. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 1/09/2015 04:18, Niall Douglas wrote:
- The documentation notes: "Don't do directory enumerations from an x86 binary on a x64 Windows system. This is due to a bug in Microsoft's WOW64 syscall translation layer which Microsoft have decided is wontfix. " Given that a major purpose of AFIO is to provide a portable abstraction that works around OS limitations, AFIO needs to support directory enumeration even in this case (e.g. by detecting WOW64 and using a synchronous call via a threadpool in that case).
This problem is caused by how ASIO's reactor works and is due to code I can't modify in ASIO. It'll go away when I rewrite ASIO's reactor using lightweight futures because I can control the source code.
The suggestion was that you explicitly detect the problem case (ie. running under WOW64) and then execute an alternate enumeration codepath in that case (avoid the problem API and call a different one, perhaps synchronously). I'm not sure how ASIO's reactor bears any relationship to this. If the problem is that you need to call a synchronous API, then note that you can queue up work via the common threadpool reactor that actually executes synchronously on a dedicated worker thread, or pool thereof (ASIO even does this itself internally for some things, where async APIs are not available). It's a little less efficient, of course, but not particularly difficult. If it's not about synchrony, then presumably you need to call the Win32 API instead of the Native API in this case. So do that. It's not acceptable to simply say "directory enumeration doesn't work in 32-bit Windows processes sometimes". That's a bug, and even if MS agreed to fix it you would still have to have code that worked around it on versions before the fix.
AFIO fatal exits the process if you try to read or write past the current file size, it's a logic error and your program is inherently broken.
FWIW, any library choosing to fatal exit the process for ANY reason causes me to fatally exit the library (ie. not use it), particularly if it doesn't provide interception hooks. My apps have their own methods for reacting to fatal errors of any kind (mostly unified logging and graceful exit) and I don't appreciate anything that tries to bypass them. (In fact I have some code specifically dedicated to preventing the VC++ CRT from trying to bypass the error handler.)
On 2015-09-01 07:34, Gavin Lambert wrote: > On 1/09/2015 04:18, Niall Douglas wrote: >>> - The documentation notes: "Don't do directory enumerations from an >>> x86 binary >>> on a x64 Windows system. This is due to a bug in Microsoft's >>> WOW64 syscall >>> translation layer which Microsoft have decided is wontfix. " >>> Given that a >>> major purpose of AFIO is to provide a portable abstraction that >>> works around >>> OS limitations, AFIO needs to support directory enumeration even >>> in this case >>> (e.g. by detecting WOW64 and using a synchronous call via a >>> threadpool in that >>> case). >> >> This problem is caused by how ASIO's reactor works and is due to code >> I can't modify in ASIO. It'll go away when I rewrite ASIO's reactor >> using lightweight futures because I can control the source code. > > The suggestion was that you explicitly detect the problem case (ie. > running under WOW64) and then execute an alternate enumeration > codepath in that case (avoid the problem API and call a different one, > perhaps synchronously). I'm not sure how ASIO's reactor bears any > relationship to this. > > If the problem is that you need to call a synchronous API, then note > that you can queue up work via the common threadpool reactor that > actually executes synchronously on a dedicated worker thread, or pool > thereof (ASIO even does this itself internally for some things, where > async APIs are not available). It's a little less efficient, of > course, but not particularly difficult. > > If it's not about synchrony, then presumably you need to call the > Win32 API instead of the Native API in this case. So do that. > > It's not acceptable to simply say "directory enumeration doesn't work > in 32-bit Windows processes sometimes". That's a bug, and even if MS > agreed to fix it you would still have to have code that worked around > it on versions before the fix. > >> AFIO fatal exits the process if you try to read or write past the >> current file size, it's a logic error and your program is inherently >> broken. > > FWIW, any library choosing to fatal exit the process for ANY reason > causes me to fatally exit the library (ie. not use it), particularly > if it doesn't provide interception hooks. + 1 And even more so in this particular case since reading past the end of a file is a recoverable error (it's not like we're deep in UB-land). @Niall: Compare this to vector::at(). Assume for a second a standard in some other universe which said: /std::vector::at() fatal exits the process if you try to read or write past the current vector size, it's a logic error and your program is inherently broken./ Would you want that? If no: Why would you want to terminate in case of reading past the end of a file? > > My apps have their own methods for reacting to fatal errors of any > kind (mostly unified logging and graceful exit) and I don't appreciate > anything that tries to bypass them. (In fact I have some code > specifically dedicated to preventing the VC++ CRT from trying to > bypass the error handler.)
On 1 Sep 2015 at 17:34, Gavin Lambert wrote:
- The documentation notes: "Don't do directory enumerations from an x86 binary on a x64 Windows system. This is due to a bug in Microsoft's WOW64 syscall translation layer which Microsoft have decided is wontfix. " Given that a major purpose of AFIO is to provide a portable abstraction that works around OS limitations, AFIO needs to support directory enumeration even in this case (e.g. by detecting WOW64 and using a synchronous call via a threadpool in that case).
This problem is caused by how ASIO's reactor works and is due to code I can't modify in ASIO. It'll go away when I rewrite ASIO's reactor using lightweight futures because I can control the source code.
The suggestion was that you explicitly detect the problem case (ie. running under WOW64) and then execute an alternate enumeration codepath in that case (avoid the problem API and call a different one, perhaps synchronously). I'm not sure how ASIO's reactor bears any relationship to this.
ASIO uses the problematic parameter to pass a pointer to some state. The segfault is caused by ASIO dereferencing that pointer.
It's not acceptable to simply say "directory enumeration doesn't work in 32-bit Windows processes sometimes". That's a bug, and even if MS agreed to fix it you would still have to have code that worked around it on versions before the fix.
It's only x86 on x64 kernels, but that is still a common enough case. I do agree, and it'll be fixed in the refactor removing ASIO.
AFIO fatal exits the process if you try to read or write past the current file size, it's a logic error and your program is inherently broken.
FWIW, any library choosing to fatal exit the process for ANY reason causes me to fatally exit the library (ie. not use it), particularly if it doesn't provide interception hooks.
My apps have their own methods for reacting to fatal errors of any kind (mostly unified logging and graceful exit) and I don't appreciate anything that tries to bypass them. (In fact I have some code specifically dedicated to preventing the VC++ CRT from trying to bypass the error handler.)
I have been thinking about this, and I do see how it would be unhelpful if a shipping product saw a bit flip from a cosmic ray and AFIO hoses the process. It would get in the way of telemetry. I may consider instead to flip a global boolean which permanently stops dead and disables AFIO in that process. That would prevent the damage of other people's data, but still allow you to send telemetry. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
participants (11)
-
Andrey Semashev -
Asbjørn -
Gavin Lambert -
Giovanni Piero Deretta -
Hartmut Kaiser -
Jeremy Maitin-Shepard -
John Bytheway -
Manu343726 -
Michael Marcin -
Niall Douglas -
Roland Bock