Hello everyone, The review of the proposed Boost Shmem library written by Ion Gaztañaga starts today (February 6th, 2006) and ends after 10 days (February 16, 2006). Documentation for the library can be found online here: http://tinyurl.com/e2os3 Both documentation and the library itself can be downloaded here: http://tinyurl.com/9t5qy --------------------------------------------------- About the library: The Shmem Library simplifies the use of shared memory and provides some STL compatible allocators that allow shared memory STL container placement. It also offers helper classes, such as offset pointers, process shared mutexes, condition variables, and named semaphores. Library users can apply all STL algorithms and utilities with created shared memory objects. Shmem also wants to present a portable implementation, unifying UNIX and Windows shared memory object creation, providing simple wrapper classes. Shmem offers the following to the user: - Portable synchronization primitives for shared memory, including shared mutexes and shared condition variables. - Dynamic allocation of portions of a shared memory segment. - Named allocation in shared memory. Shmem can create objects in shared memory and associate it with a c-string offering a similar mechanism to a named new/delete. Created object can be found by other processes using Shmem framework. - Offset pointers that can be safely placed in shared memory to point to another object of the same shared memory segment even if the memory is mapped to a different address. - STL compatible shared memory allocators so that STL containers can be placed in shared memory. Shmem also offers a pooled node allocator to save and optimize shared memory allocation in node containers. - STL compatible containers for systems with STL implementations that cannot deal with shared memory. The user can map shared memory to different addresses and use STL compatible containers and algorithms. Useful to store common data or implement shared memory databases. These containers also show how STL compatible containers can make use of shared memory STL compatible allocators to place such containers in the shared memory. Shmem also offers the basic_string pseudo-container to use full-powered C++ strings in shared memory. --------------------------------------------------- Please always state in your review, whether you think the library should be accepted as a Boost library! Additionally please consider giving feedback on the following general topics: - What is your evaluation of the design? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - What is your evaluation of the potential usefulness of the library? - Did you try to use the library? With what compiler? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain? Fred Bertsch Review Manager
G'day everyone. Just in case I don't get a chance to do a formal review before the 16th, I have one brief comment. The mmaped_file class merges "a file which has some of its space mapped" with "a mapped portion of a file". While I realise that this is a shared memory library, not a general memory mapped file library, it might be useful to separate the two. The problem is that under the current design, mapping two separate parts of one file (a common task when trying to be clever with page-structured files) requires two file descriptors and two filename lookups. The other option is mapping a region which includes both parts, which wastes address space (occasionally to the point of it being impossible; consider mapping a 4Gb file on a 32-bit architecture). Cheers, Andrew Bromage
Hi Andrew,
Just in case I don't get a chance to do a formal review before the 16th, I have one brief comment.
I hope you can find to do a review, since it seems you have experience with memory mappings.
The mmaped_file class merges "a file which has some of its space mapped" with "a mapped portion of a file". While I realise that this is a shared memory library, not a general memory mapped file library, it might be useful to separate the two.
I'm open to develop a more advanced memory mapped file. I have no experience with memory mapping but if you want to help, I'm ready to put some effort.
The problem is that under the current design, mapping two separate parts of one file (a common task when trying to be clever with page-structured files) requires two file descriptors and two filename lookups. The other option is mapping a region which includes both parts, which wastes address space (occasionally to the point of it being impossible; consider mapping a 4Gb file on a 32-bit architecture).
If I understand this correctly, you want a class that can map more than a fragment of a file holding a single file descriptor, is that right? That would require that the memory mapped class should have a list of mappings (to unmap them in the destructor, for example). I think we should define about the interface you would want for this class. It can be implemented in a different class than current (simple) boost::shmem::mmapped_file. I don't have experience with this type of multi-mapped files but it's surely a good idea. Regards, Ion
G'day Ion. Ion Gaztañaga <igaztanaga <at> gmail.com> writes:
I hope you can find to do a review, since it seems you have experience with memory mappings.
I hope so too. :-)
If I understand this correctly, you want a class that can map more than a fragment of a file holding a single file descriptor, is that right?
Well... I want a way to do that, whether it's a single class or not. When I first thought about it, the approach that I considered was to use two classes. mmaped_file represents a file which has zero or more mmaped regions, and mmaped_region which represents a single region. Off the top of my head, something like this might be appropriate: class mmapped_file : private boost::noncopyable { public: /* A slightly richer set of access modes. If the operating system does not support some mode, the library should pick a mode that at least allows the required mode. (e.g. if the OS doesn't support write-only mode, read-write mode is an appropriate substitute since it allows writing. */ typedef enum { none_mode = 0x00, ro_mode = 0x01, wo_mode = 0x02, rw_mode = 0x03, access_mode_mask = 0xff } accessmode; /* Open a file, RAII style. Throws an exception on failure. */ mmapped_file(const char* filename, accessmode mode); /* Closes the file. Does not throw. */ ~mmapped_file(); /* XXX Support for iterating through the mapped regions? */ }; class mmaped_region : private boost::noncopyable { public: /* Map a region from an mmaped_file. Throws an exception on failure. */ mmaped_region(shared_ptr<mmaped_file>& file, fileoff_t offset, size_t size, mmaped_file::accessmode mode); /* Unmap the region. Does not throw. */ ~mmapped_region(); /* Get the size of the mapped region. Does not throw. */ std::size_t get_size() const; /* Get a pointer to the base of the mapped region. Does not throw. */ void* get_base() const; /* Get the file offset of the beginning of the mapped region. Does not throw. */ fileoff_t get_file_offset() const; /* Flush an area of the mapped region. Does not throw. */ bool flush(size_t mapping_offset = 0, size_t numbytes = 0); }; mmaped_region holds a shared_ptr to an mmaped_file, so that when the last mapped region is destroyed, the file is closed (assuming someone else isn't holding onto if, of course). Cheers, Andrew Bromage
This seems like a good seperation of the concepts (file vs. region) to me. Most of the higher level functionality in shmem could be built against the mmapped_region class. I definitely feel that this is a step in the right direction. Dave Moore When I first thought about it, the approach that I considered was to use
two classes. mmaped_file represents a file which has zero or more mmaped regions, and mmaped_region which represents a single region. Off the top of my head, something like this might be appropriate:
class mmapped_file : private boost::noncopyable { public: /* A slightly richer set of access modes. If the operating system does not support some mode, the library should pick a mode that at least allows the required mode. (e.g. if the OS doesn't support write-only mode, read-write mode is an appropriate substitute since it allows writing. */ typedef enum { none_mode = 0x00, ro_mode = 0x01, wo_mode = 0x02, rw_mode = 0x03, access_mode_mask = 0xff } accessmode;
/* Open a file, RAII style. Throws an exception on failure. */ mmapped_file(const char* filename, accessmode mode);
/* Closes the file. Does not throw. */ ~mmapped_file();
/* XXX Support for iterating through the mapped regions? */ };
class mmaped_region : private boost::noncopyable { public: /* Map a region from an mmaped_file. Throws an exception on failure. */ mmaped_region(shared_ptr<mmaped_file>& file, fileoff_t offset, size_t size, mmaped_file::accessmode mode);
/* Unmap the region. Does not throw. */ ~mmapped_region();
/* Get the size of the mapped region. Does not throw. */ std::size_t get_size() const;
/* Get a pointer to the base of the mapped region. Does not throw. */ void* get_base() const;
/* Get the file offset of the beginning of the mapped region. Does not throw. */ fileoff_t get_file_offset() const;
/* Flush an area of the mapped region. Does not throw. */ bool flush(size_t mapping_offset = 0, size_t numbytes = 0); };
When I first thought about it, the approach that I considered was to use two classes. mmaped_file represents a file which has zero or more mmaped regions, and mmaped_region which represents a single region. the Off top of my head, something like this might be appropriate:
I see. Another possibility is a "mmapped_file" class that produces "file_mappings". However this has not the lifetime security your approach has, since your mapped_file won't be destroyed until the last mapped_region is destroyed. However, being from the old school, I don't like much the mandatory use of shared_ptr in the interface (so that mapped_file must be allocated only via new). Anyway, I get the idea of two separate concepts and I can see that your approach saves a lot of lifetime issues, and I don't think file_mappings will be objects that will be created and destroyed very heavily. file_mappings could have also ordering operators so that we can store them in containers. I think both your mapping approach and boost::shmem::mmapped_file can be complementary, being boost::shmem::mmapped_file for simple uses and this multi-mapping approach for advanced ones. If you want to implement it, I'm ready to help or if you just want to help, I'm ready to implement it for a future Shmem version. Thanks, Ion
G'day. Ion Gaztañaga <igaztanaga <at> gmail.com> writes:
However, being from the old school, [...]
So why are you proposing a Boost library? :-) (I hope that emoticon was big enough!)
I don't like much the mandatory use of shared_ptr in the interface (so that mapped_file must be allocated only via new).
That's not true! You can take a shared_ptr to a static or auto object by using a custom deleter. This approach doesn't provide any safety in this case (since mmaped_region doesn't manage the lifetime of mmaped_file), but it works at least.
file_mappings could have also ordering operators so that we can store them in containers.
Yes. The application that I had in mind was an application-controlled buffer cache, so the ability to store them in containers is certainly a useful feature there.
I think both your mapping approach and boost::shmem::mmapped_file can be complementary, being boost::shmem::mmapped_file for simple uses and this multi-mapping approach for advanced ones.
Yes. In fact, the more I think about it, the more I suspect that this is the better approach, with mmaped_file perhaps being a convenience wrapper built on top of the other two classes. In addition, while designing these classes, it may turn out that a more general memory mapped file library is warranted. So in summary: We (I include myself) should work on supporting this functionality, but it's not a barrier to accepting Shmem into Boost. Cheers, Andrew Bromage
Hi Andrew,
However, being from the old school, [...]
So why are you proposing a Boost library? :-)
Good point! Maybe because I felt old-school resources (IPC, shared memory, etc...) are not present in Boost ;-)
I don't like much the mandatory use of shared_ptr in the interface (so that mapped_file must be allocated only via new).
That's not true! You can take a shared_ptr to a static or auto object by using a custom deleter. This approach doesn't provide any safety in this case (since mmaped_region doesn't manage the lifetime of mmaped_file), but it works at least.
Right.
file_mappings could have also ordering operators so that we can store them in containers.
Yes. The application that I had in mind was an application-controlled buffer cache, so the ability to store them in containers is certainly a useful feature there.
The buffer cache is definitely a good tool to design. Don't forget two present it to a Boost review!
Yes. In fact, the more I think about it, the more I suspect that this is the better approach, with mmaped_file perhaps being a convenience wrapper built on top of the other two classes.
Yes, that would be easy.
In addition, while designing these classes, it may turn out that a more general memory mapped file library is warranted.
So in summary: We (I include myself) should work on supporting this functionality, but it's not a barrier to accepting Shmem into Boost.
Thanks. I think that multi-mapping functionality has many possibilities, so is definitely a good project. Cheers, Ion
Hello, I've played around with shmem library, and I was really impressed with it power, design and functionality. I definitely think this library should be accepted into boost.
- What is your evaluation of the design? In general - I liked it. It looks a bit complicated in some places, but I guess this is tradeoff played for its power. I've made several tests and found that library is easy enough to use, yet powerful enough to do what most people need for IPC communications.
One minor thing, I didn't particularly liked about design is that several method (for example shared_message_queue::receive) return values into variables passed by non-const reference. I don't like this approach - the resulting code is obscure because you can't see from that code, that variable is modified. I think of the the following approaches might result a better client code: 1) Return multiple values (i.e. by using std::pair, boost::tuple) 2) Modify parameters, but instead of passing by non-const reference - pass them by pointer. When pointer passed it immediately indicates that variable might be modified as opposite to non-const reference, which is a rare beast nowadays. If you think of foo(someVar) via foo(&someVar), the second hints about possibility of modifying someVar, while first one doesn't.
- What is your evaluation of the implementation? I did not looked into implementation.
- What is your evaluation of the documentation? The documentation is good and comprehensive. I likes a large number of small and explaining code examples.
Here, however, are the few glitches I've came by: 1) in file containers_explained.html "boost:shmem" written several times instead of "boost::shmem" 2) in file named_shared_object.html Document states that basic_named_shared_object::open in has 3 parameters while open in reality it has only two (no size is passed to open) 3) Most of the pages missing includes files needed for particular class/feature. It would be nice, if, for example, shared_message_queue.html would mention <boost/shmem/ipc/shared_message_queue.hpp>
- What is your evaluation of the potential usefulness of the library? Very useful. I've seen few libraries what tried to deal with the shared memory, but non got even close to level of functionality the shmem library provides.
- Did you try to use the library? With what compiler? Did you have any problems? I've tried using VC 7.1 and boost 1.33.1. I did compiled and run without a single problem.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I've read most of the documentation, and played around with library. Wrote few small programs to test some of the features, but didn't use in any large project.
- Are you knowledgeable about the problem domain? Moderate. I didn't work with IPC a lot, but deal with it from time to time.
And I would like to thank Ion Gaztañaga for developing such a good library. Hope it will make its way to the proud family of boost libraries :)
Pavel Antokolsky aka Zigmar wrote:
- What is your evaluation of the design? In general - I liked it. It looks a bit complicated in some places, but I guess this is tradeoff played for its power. I've made several tests and found that library is easy enough to use, yet powerful enough to do what most people need for IPC communications.
One minor thing, I didn't particularly liked about design is that several method (for example shared_message_queue::receive) return values into variables passed by non-const reference. I don't like this approach - the resulting code is obscure because you can't see from that code, that variable is modified. I think of the the following approaches might result a better client code: 1) Return multiple values (i.e. by using std::pair, boost::tuple) 2) Modify parameters, but instead of passing by non-const reference - pass them by pointer. When pointer passed it immediately indicates that variable might be modified as opposite to non-const reference, which is a rare beast nowadays. If you think of foo(someVar) via foo(&someVar), the second hints about possibility of modifying someVar, while first one doesn't.
I have not had time to look into shmem yet, so my comment is only to the above text without looking at the code. 1) This is not always a good idea because of the cost of copying a returned object. Also when using templated functions the compiler can deduce the type from the argument instead of having to specify it explicitly. 2) No, No, No. I strive for pointer free code. A method that advertises a non-const reference tells you that it will modify the object. A const reference indicates that it will not. A function that takes a pointer tells me nothing. KevinH -- Kevin Heifner heifner @ ociweb.com http://heifner.blogspot.com Object Computing, Inc. (OCI) www.ociweb.com
I have not had time to look into shmem yet, so my comment is only to the above text without looking at the code. 2) No, No, No. I strive for pointer free code. A method that advertises a non-const reference tells you that it will modify the object. A const reference indicates that it will not. A function that takes a pointer tells me nothing. The problem is with calling code - you can no tell if the call is by const reference or non-const reference, until you find and check function declaration. And you might not know at all that there is such
On 2/9/06, Kevin Heifner <heifner_k@ociweb.com> wrote: possibility. foo(var) give no clue of the possibility of "var" been modified. In any case it is a controversial point - pointer are bad because they are pointer and we don't like pointers in clean C++ code, besides they may take NULL value. Non-const references are obscure in client code, but really fast, and returning multiple values, are seems to be the "cleanest" way, but the least efficient. -- Best regards, Zigmar
On 2/9/06, Pavel Antokolsky aka Zigmar <zigmar@gmail.com> wrote:
One minor thing, I didn't particularly liked about design is that several method (for example shared_message_queue::receive) return values into variables passed by non-const reference. I don't like this approach - the resulting code is obscure because you can't see from that code, that variable is modified. I think of the the following approaches might result a better client code: 1) Return multiple values (i.e. by using std::pair, boost::tuple) 2) Modify parameters, but instead of passing by non-const reference - pass them by pointer. When pointer passed it immediately indicates that variable might be modified as opposite to non-const reference, which is a rare beast nowadays. If you think of foo(someVar) via foo(&someVar), the second hints about possibility of modifying someVar, while first one doesn't.
I'd like to second KevinH on this one; I think that a non-const reference is a much better choice than a pointer. To me, non-const reference means output while a pointer means that the method will be taking ownership of that pointer. ~ Scott
me22 wrote:
On 2/9/06, Pavel Antokolsky aka Zigmar <zigmar@gmail.com> wrote:
When pointer passed it immediately indicates that variable might be modified as opposite to non-const reference, which is a rare beast nowadays. If you think of foo(someVar) via foo(&someVar), the second hints about possibility of modifying someVar, while first one doesn't.
That sounds like a local convention to me. Actually it sounds like something a C programmer would think to me. Why pass anything by pointer unless you have to?
To me, non-const reference means output
Me too.
while a pointer means that the method will be taking ownership of that pointer.
Well, it's a possibility, but again not necessarily global law I think. j
Johan Johansson wrote:
That sounds like a local convention to me. Actually it sounds like something a C programmer would think to me. Why pass anything by pointer unless you have to?
I think Bjarne Stroustrup expressed this sentiment once, although I personally disagree. The reason is simple: call-by-reference and call-by-value have no visible difference at the call site, which could therefore make changes to the arguments unexpected. Sebastian Redl
On 2/9/06, Sebastian Redl <sebastian.redl@getdesigned.at> wrote:
Johan Johansson wrote: The reason is simple: call-by-reference and call-by-value have no visible difference at the call site, which could therefore make changes to the arguments unexpected.
supposing std::fstream myfile: int i; myfile >> i; myfile << i; char c; myfile.get(c); It's not "myfile >> &i;", nor is it "myfile.get(&c);", and yet you understand fine. If I know what a function does--which is hopefully fairly obvious from its name--then I know which are output parameters, and the & is just an annoyance. If I don't know what the function does, then I'll have to look it up in documentation to find out, at which point I'll learn what the parameters are for. The & might be slightly useful were I trying divination to figure out what the function does, but even without that most non-gargantuan functions make it fairly clear which are output parameters from context. ~ Scott
On 2/9/06, Johan Johansson <johanj@ipunplugged.com> wrote:
while a pointer means that the method will be taking ownership of that pointer.
Well, it's a possibility, but again not necessarily global law I think.
Well, it's obviously not a "global law". But what does pass-by-pointer give over pass-by-reference? There's the possibility of null, but I'd rather do optional arguments with overloaded functions or, for complicated ones, with Boost.Parameter. The only other difference is in ownership/lifetime management ( though I'd rather make that more explicit with an std::auto_ptr or other smart pointer anyways ). I basically don't see any other reason to pass by pointer except for lifetime, which is irrelevant when it's being used as an output parameter since there's no resource ownership transfer. ~ Scott
me22 wrote:
On 2/9/06, Johan Johansson <johanj@ipunplugged.com> wrote:
while a pointer means that the method will be taking ownership of that pointer.
Well, it's a possibility, but again not necessarily global law I think.
Well, it's obviously not a "global law".
But what does pass-by-pointer give over pass-by-reference? There's the possibility of null, but I'd rather do optional arguments with overloaded functions or, for complicated ones, with Boost.Parameter. The only other difference is in ownership/lifetime management ( though I'd rather make that more explicit with an std::auto_ptr or other smart pointer anyways ). I basically don't see any other reason to pass by pointer except for lifetime, which is irrelevant when it's being used as an output parameter since there's no resource ownership transfer.
I'm not arguing that pass-by-pointer is preferable. I'm actually arguing the opposite. I'm just saying that just as pass-by-pointer doesn't signal "the argument may change" I'm not convinced it signals "I'll delete the argument". As for the smart pointer option, I actually wrote a response to that effect but I was kind of expecting to get slapped around with a "but I don't want to use smart pointers" and deleted it... j
Hi Pavel,
I've played around with shmem library, and I was really impressed with it power, design and functionality. I definitely think this library should be accepted into boost.
Thanks.
Here, however, are the few glitches I've came by:
1) in file containers_explained.html "boost:shmem" written several times instead of "boost::shmem"
Thanks for pointing it out.
2) in file named_shared_object.html Document states that basic_named_shared_object::open in has 3 parameters while open in reality it has only two (no size is passed to open)
Thanks!
3) Most of the pages missing includes files needed for particular class/feature. It would be nice, if, for example, shared_message_queue.html would mention <boost/shmem/ipc/shared_message_queue.hpp>
Ok, you are right. I will add the header which implements every class.
And I would like to thank Ion Gaztañaga for developing such a good library. Hope it will make its way to the proud family of boost libraries :)
I hope so! Thanks for reviewing the library. Ion
Pavel Antokolsky aka Zigmar <zigmar@gmail.com> writes:
2) Modify parameters, but instead of passing by non-const reference - pass them by pointer. When pointer passed it immediately indicates that variable might be modified as opposite to non-const reference, which is a rare beast nowadays. If you think of foo(someVar) via foo(&someVar), the second hints about possibility of modifying someVar, while first one doesn't.
One problem with that is that by convention, pointer arguments can be null, whereas the language says there are no legally-formed null references. That said, I agree that "out" parameters should be used sparingly at best. -- Dave Abrahams Boost Consulting www.boost-consulting.com
participants (10)
-
Andrew J Bromage -
Dave Moore -
David Abrahams -
fred＠styleadvisor.com -
Ion Gaztañaga -
Johan Johansson -
Kevin Heifner -
me22 -
Pavel Antokolsky aka Zigmar -
Sebastian Redl