Re: [boost] Proposal: Monotonic Containers
Hi, Several notes: 1st... This code does not compile with gcc-4.3 (mingw) a) Fix: map.h/set.h --> - set(Predicate P, Allocator const &A) - : Set(P, A) + set(Predicate Pr, Allocator const &A) + : Set(Pr, A) because it shadows template parameter P. b) Fix in ./boost/monotonic/storage_base.h Add #include <boost/cstdint.hpp> 2nd... Never use: #pragma once not portable 3rd... Your allocator is not correct you do not align the pointers as required by most of architectures. You need to provide 8-16 bytes alignment... According to system 4th... Why don't you use boost::pool? It is debugged and tested instead of writing new implementation? 5th... Implement ALL constructors of each container. For example I can call. std::string str="Foo"; std::vector<int> v(str.begin(),str.end()); this is not supported 6th... This is bad idea to derive from std containers. They to do not provide virtual destructors. This may be very error prone. This is just short glance. Artyom --- On Tue, 6/9/09, Christian Schladetsch <christian.schladetsch@gmail.com> wrote:
From: Christian Schladetsch <christian.schladetsch@gmail.com> Subject: Re: [boost] Proposal: Monotonic Containers To: boost@lists.boost.org Date: Tuesday, June 9, 2009, 3:06 PM Hello,
busy people, and anything that takes extra time is
That is not what I asked for. I wanted online docs and code. We are very likely to mean we don't
bother.
OK well on second thought, what?
I am also busy.
I gave you a link to code and documentation in the one archive. http://www.boostpro.com/vault/index.php?action=downloadfile&filename=MonotonicAllocator_dox.zip&directory=&
Click on the link, get the code and the documentation. Sorry that you have to click into the archive twice, but boost doesn't have a better way of doing it ATM that I am aware of.
Chrstian. _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hi Artyom, [snip] ...because it shadows template parameter P. Fixed.
b) Fix in ./boost/monotonic/storage_base.h Add #include <boost/cstdint.hpp>
Fixed.
3rd... Your allocator is not correct you do not align the pointers as required by most of architectures. You need to provide 8-16 bytes alignment... According to system
I will think more about this. My initial impression was that "I am not doing any allocation, so I do not need to do any alignment". However, I am returning pointers to objects (even if those pointers are all in a static buffer) and yes they should be aligned.
4th... Why don't you use boost::pool? It is debugged and tested instead of writing new implementation?
boost::pool does not provide a means to use STL with stack-based memory.
5th... Implement ALL constructors of each container. For example I can call. std::string str="Foo"; std::vector<int> v(str.begin(),str.end()); this is not supported
Yes, I have not fully implemented all the various ctor overloads for the various containers. Each of them simply needs to take the extra argument of the storage (or the allocator). I will add them.
6th... This is bad idea to derive from std containers. They to do not provide virtual destructors. This may be very error prone.
I agree that it is generally a bad idea to derive from std containers. However, it seems that one would have to purposefully write pathological code in order for this to be a problem in this case. Using inheritance means that existing code that pattern-matches against the std::containers, will also pattern-match against monotonic::containers.
This is just short glance.
Artyom
Thanks a lot for your help and suggestions. I'll consider more about alignment issues. Regards, Christian.
Hi Artyom, Thanks a lot for your help and suggestions. I'll consider more about
alignment issues.
It cannot be the case that the monotonic::allocator has to align results. The space has already been created before the allocator is requested to provide for new storage. Otherwise, we could never use a vector<char> on a machine that requires 16-byte alignment. Regards, Christian.
participants (2)
-
Artyom -
Christian Schladetsch