- Boost - lists.boost.org

[poly_collection] Memory allocation and allocator propagation
by Ion Gaztañaga 17 May '17

17 May '17

Hi Joaquín, Some additional comments about the design: ///////////////////////// // Memory indirections ///////////////////////// If I understand code correctly, the internal structure of a poly collection is approximately: 1) poly_collection stores an unordered_map<type_index, segment_type> 2) segment_type stores a unique_ptr<segment_backend> and segment_backend_is an abstract class, so roughly equivalent to unique_ptr<packed_segment/split_segment> (both packed_segment/split_segment derive from segment_backend. 3) packed_segment/split_segment stores a vector of concrete types. So we need 3 memory hops to navigate between the "this" pointer of a PolyCollection to a pointer to a concrete type: poly_collection -> unordered node -> segment in unique_ptr -> vector value. I think the data structure could be optimized to save one indirection. segment_type could directly hold the class derived from segment_backend as packed_segment/split_segment will need the same size and alignment requirements regardless of the ConcreteType they hold (as long as they use std::vector). This could improve insertion times and other operations. This optimization could also make easier to fix the following section (allocator propagation). *Main question 1*: Do you find think PolyCollection should implement this optimization? ///////////////////////// // Allocator propagation ///////////////////////// As mentioned in Adam Wulkiewicz's review PolyCollection currently does not propagate the allocator from the constructor argument of poly_collection to the concrete type. From a practical point of view, the key requirement is that all intermediate memory until the concrete type must be allocated using the same root allocator passed when a poly_collection was constructed, and that allocator must be propagated also to the concrete type. I think this should also be the case of all memory used for auxiliary data like the index vector of split_sequence. Clause 23.2.1.3 was mentioned although I don't think this clause was designed with compound containers in mind, as IMHO all auxiliary data, including metadata that needs dynamic memory should receive the allocator and should be constructed using allocator_traits or a container that uses allocator_traits (yes, even the non-value parts of a node). Otherwise, the original intention of the new standard allocator model (use a single source of memory and propagate it transparently) wouldn't be feasible. I think that a unique memory source for all internal structures and the correct propagation of the allocator using allocator_traits is a very interesting feature for PolyCollection. Even Bjarne Stroustrup commented on this when Joaquín's article was published in isocpp ( https://isocpp.org/blog/2014/05/fast-polymorphic-collections-joaquin-m-lope…) "One thing caught my eye: The elements could be allocated using stack allocators, rather than a general new. That would make the technique applicable to hard real-time systems where general dynamic memory is banned" *Main question 2*: Do you think PolyCollection should support stateful allocator using the scoped allocator model? Some specific notes that might show places where the scoped allocator model has problems.: - PolyCollection uses operator new to allocate the packed/split_segment implementation stored in unique_ptr and also when the segment must be copied. If unique_ptr is avoided and the concrete segment is in-place constructed inside "segment" then all memory will be allocated from the original allocator. - I think "segment" is missing an extended copy and move constructor taking an additional allocator. It also would need an internal allocator_type typedef so that uses_allocator_v is true and unordered_map calls these new constructors when needed. These new constructors should be propagated to split_segment/packed_segment so that these call the extended constructors of internal vectors. - The concrete type wrappers (e.g. value_holder) also would need to support allocators with extended copy/move constructors. - The "emplace" operation uses a placement new to wrapped in a lambda which is type-erased with a captureless lambda. It looks to me that instead of delegating the work into the value_holder, the type erasure arguments should be treated by the segment so that internal vectors propagate the allocator when needed. A test to make sure everything is correctly propagated: -> Create an instance of monotonic_buffer_resource passing a big enough static buffer. -> Create a base_collection< Base, std::polymorphic_allocator<Base> > passing the address of the monotonic_buffer_resource -> Insert types Derived1 and Derived2 in the container, both of which hold a vector<int, polymorphic_allocator<int> > (for Derived1 and Derived2 are uses_allocator_v is true and they propagate the memory resource to the vector). -> After filling base_collection, no heap allocation is made and all objects are constructed in the static buffer. Best, Ion

3 8

Re: [boost] Hello Again From Facebook Engineering
by Yurii Zubrytskyi 16 May '17

16 May '17

Hi Scott, I appreciate the email. Things are going great here, and I'm really happy with Google. Anyway, even if I wanted to change jobs I wouldn't consider Facebook until it opens an office on the Eastside - commuting into Seattle is just too uncomfortable, so I'm not considering any company there Thanks! ________________________________ From: Scott Shay <scottshay(a)fb.com> Sent: Tuesday, April 18, 2017 10:03:15 AM To: Yuriy Zubritsky; boost(a)lists.boost.org Subject: Hello Again From Facebook Engineering Hi Yuriy, Sending a note to say hello again from Facebook! My great teammate, Camila, mentioned that I should reach out and see how things at Google have been going for you? Have you had some time to consider what's next in your career? I understand if this is not the right timing, kindly let me know how we can best keep in touch for when it makes more sense. Thanks a lot and I look forward to hearing back from you. P.S. in case you're curious, here's a resource that highlights some of the technical problems we tackle. https://code.facebook.com/ Scott Shay Engineering Sourcer | Recruiting M: (650) 785-6142 [id:image001.png@01D298C9.4BBDBCE0] facebook.com/careers<https://www.facebook.com/careers>

1 0

crc // wrong poly for crc_xmodem_type
by Sergei Nikulov 16 May '17

16 May '17

Hello All, I've discovered that current typedef crc_xmodem_type uses wrong poly. Current release and master branch: > typedef crc_optimal<16, 0x8408, 0, 0, true, true> crc_xmodem_type; Should be (ref: http://reveng.sourceforge.net/crc-catalogue/16.htm#crc.cat-bits.16) > typedef crc_optimal<16, 0x1021, 0, 0, false, false> Develop branch already have some changes which address this error (for ex. https://github.com/boostorg/crc/commit/328bf50f666c463901e2179aa2d890916c3b…) but master and boost release does not. Who currently owns boost::crc and can fix this issue? Thank you. -- Best Regards, Sergei Nikulov

2 2

[Convert] convert<float> doesn't respect input size
by Olaf van der Spek 15 May '17

15 May '17

Hi, const char* a = "42"; boost::string_ref b(a, 1); float c = boost::convert<float>(b, boost::cnv::strtol(), -1); // returns 42 assert(c == 4); https://github.com/boostorg/convert/issues/29 -- Olaf

1 0

Re: [boost] [http] Boost.Http review report
by Vinnie Falco 15 May '17

15 May '17

On Aug 27, 2015 at 12:21 PM, Bjorn Reese <breese_at_[hidden]> wrote: > The outcome of the formal review of Boost.Http is that the library is > NOT accepted for inclusion into Boost this time. > ... > Judging from the reviews, the following additions will > make a future submission more likely to succeed: > > 1. Involve the community more during the development. This will > both solicit earlier feedback on the design, and help to align > community expectation. I've been trying to involve the community for several months now, offering opportunities to weigh in on design decisions, provide feedback on pull requests, perform code reviews on pending merges. The response has been, to put it nicely, minimal. In particular I have not heard from anyone involved in the other proposed libraries such as Boost.Http or cpp-netlib except some initial contacts in early 2016.

2 3

Re: [boost] [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)
by Bjorn Reese 14 May '17

14 May '17

On 05/11/2017 06:19 PM, charleyb123 . via Boost wrote: > GitHub: > https://github.com/ned14/boost.outcome The code has a "boost-lite" submodule. Is that part of the review?

5 13

Win10 now allows symlink creation without elevated privileges
by Niall Douglas 14 May '17

14 May '17

In case the list didn't know, from the Creator's Update onwards Windows 10 allows normal users to create symlinks if the machine is put into Developer mode: https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/ I don't know about others, but this will be a major time saver for me at least, I had been using hard links on Windows which are prone to accidental data loss. Niall

5 4

[poly_collection] User-defined polymorphism model
by Ion Gaztañaga 14 May '17

14 May '17

Hi Joaquín and all reviewers, Before we reach the end of the review period, I wanted to comment an (IMHO) interesting feature used in the internal design of PolyCollection. The library offers three of polymorphism models: base-derived, boost::any and signature-sharing callable types. Under the hood, the library abstracts those polymorphic models into a generic class in the detail namespace: template<typename Model,typename Allocator> class poly_collection; and each concrete container is just implemented in terms of poly_collection: template<typename Base,typename Allocator> class base_collection : public detail::poly_collection<detail::base_model<Base>,Allocator> The first template parameter (Model) is the abstraction ("concept") that implements the non-generic part of each polymorphism model. A model implements the following types and operations (comments on each attribute are my interpretation, they can be completely wrong): template<class ModelArg> class Model { // //Value-related attributes // //The type we want to expose as value_type of the container using value_type= ...; //Is T a subtype of the general type (acceptable as value)? template<class T> using is_subtype= ... //Is T the "most refined type"? template<class T> using is_terminal=... //Id of a value template<class T> static const std::type_info& subtypeid(const T& x); //Obtain a type-erased pointer to the value template<class T> static void* subaddress(T& x); template<typename T> static const void* subaddress(const T& x); // //Segment-related // //Segment iterators that point to type_erased value_types using base_iterator=... using const_base_iterator=... //const<->non-const conversions static base_iterator nonconst_iterator(const_base_iterator it); template<typename T> static iterator<T> nonconst_iterator(const_iterator<T> it); //Concrete iterators for Derived template<typename Derived> using iterator=...; template<typename Derived> using const_iterator=... //Generic type, comparable with base_iterator. //base_iterator == the last returned base_sentinel //means that base_iterator points one-past the last //element of a segment. using base_sentinel=...; using const_base_sentinel=... //Construct a segment for type T using the given allocator and //return it type-erased. template<typename T,typename Allocator> static detail::segment_backend<Model>* make(const Allocator& al); }; I have several questions on this polymorphism model. 1) Wouldn't be a good idea to make this model-based polymorphism (or a refined one) public? Maybe the current model needs more work to support additional polymorphism models. Of course requirements for each operation in a model should be detailed. A correct abstraction to define user-provided polymorphism models is hard but this seems really a great feature that makes this library more valuable for users. 2) If understand this model abstraction correctly, each Model chooses the segment type to store values (it could be any sequence type, I guess). The segment_backend abstraction does not only abstract ConcreteTypes from value_types (using void pointers that will be casted to real types in the implementation of the virtual function), but also the segment type (the sequence holding the concrete values). Current implementation uses two segment types: split_segment (stores Concrete types and generic value_type's in two separate vectors) and packed_segment (stores ConcreteTypes which already contain value_type's as a subobject). function_collection and any_collection use the first segment, base_collection uses the second. Why are different segment types needed? Can't we store value_types and ConcreteTypes in a struct and use only one type of segment? If this is possible, I guess iteration performance would be improved for function_collection and any_collection. Model definition could be simplified. On the other hand, if the sequence type can be left out of the model definition, the underlying sequence type (change vector with deque, small vector or any other) could be made configurable. Would this be useful for users? 3) I understand that caching a base_sentinel in each segment is an optimization to avoid a virtual function call when iterating with base_iterators. Is this assumption correct? Why is a different base_sentinel type needed to mark the end of each segment? Wouldn't a "base_iterator" pointing to that on-past-the end position suffice? Is an optimization (maybe base_sentinel is cheaper) or a must have? Thanks, Ion PD: I have more design questions related to other topics but I will post a different thread for each one.

2 2

Re: [boost] [review] Review of PolyCollection starts today (May 3rd)
by Glen Fernandes 13 May '17

13 May '17

Gavin Lambert wrote: > On that note, do you know of a more-comprehensible-than-standardese guide > on how allocator_traits is supposed to be used for node-based custom > container types? I've tried to figure it out a couple times but I'm sure I'm > missing important points. What do you want to know? 1. The storage for the node should be allocated with the Allocator's 'allocate()', for an allocator instance 'a' whose value_type is the containers node type (i.e. 'a' can be achieved by rebinding an existing allocator instance in the normal way). 2. The node object itself would be constructed in the normal way - e.g. placement new to construct the node object. 3. The containers' value_type object within the node must be constructed by allocator_traits<U>::construct(u, p, args...) where u of type U is a rebound copy of the allocator such that the allocator's value_type is the container's value_type. 4. Similar to 3, for destruction of the container's value_type object should be destroyed by allocator_traits<U>::destroy(u, p) 5. Similar to 2, the node object is destroyed in the normal way - e.g. invoking the destructor of the node type. 6. Similar to 1, the storage for the node is deallocated with the Allocator's deallocate() (for an allocator instance whose value_type is the container's node type). Glen

1 0

[poly_collection] Andrzej's review
by Andrzej Krzemienski 12 May '17

12 May '17

Hi Everyone, Some comments from me about PolyCollection library, First, Joaquín, thank you for writing and sharing this library. I am not an an expert in the domain. In my programs I never needed this optimization. But I have seen it described in a number of publications, and I am convinced it deserves a place in Boost libraries, so that people are not forced to reinvent it. [1] Now, because I have no practical experience with the problem, what I say in this paragraph might be incorrect. But I have a problem here. You will use this library for improved performance, maybe for a game implementation, yet because a game needs super-performance, can a programmer afford to use dynamic dispatch, or RTTI or exceptions? I always imagined taht such library, rather than resembling `ptr_vector<I>` would resemble `vector<variant<A, B, C>>`, that is: I decide and fix the container on a set of types I will be storing inside. This set of tyes is embedded in the type, so I can detect many mis-usages at compile time. The implementation can be faster, because the number of segments is fixed (no segment management), and no registration checks need to be performed. Then, iterate over such collection with a static_visitor (from Boost.Variant). Of course, this would be constraining: I need to know all the types I will be using ahead of time. But maybe in practice this is often the case? If you disagree, maybe it is wort discussing it in the rationale. [2] I am not comfortable with per-segment functions having the same name as container-wide functions, and being only overloaded based on function or template parameters, like `cc.size()` and `cc.size(index)`. These two functions do different things, so they deserve different names. Maybe `cc.segment_size(index)`, or `cc.serment(index).size()`? [3] I downloaded it and tried toy examples with GCC 6.3 on Windows with -std=c++11, and clang 3.8.1 on Fedora with -std=c++11. It compiIes fine. I observed that the following small program crashes (assertion fails): ``` #include <boost/poly_collection/base_collection.hpp> struct Iface { Iface() = default; virtual ~Iface() = 0; }; inline Iface::~Iface() {} struct Type1 : Iface { Type1() = default; Type1(Type1&&) = delete; }; struct Type2 : Iface { Type2() = default; Type2(Type2&&) {} // throwing move // no move assignment }; int main () try { boost::base_collection<Iface> c; c.insert(Type1{}); // fires an assert c.insert(Type2{}); // fires an assert } catch (std::exception const& e) { } ``` Admittedly, `Type1` and `Type2` are not "acceptable", but according to the documentation this should throw an exception upon insertion rather than firing assertions. [4] The documentation is good. As to implementation, I did not look at it in detail. Regards, &rzej;

2 2