Re: [Boost-users] Flyweight: wrapping shared_ptr
De: Boost-users [mailto:boost-users-bounces@lists.boost.org] En nombre de Akim Demaille Enviado el: martes, 07 de octubre de 2014 18:27 Para: Boost Users List Asunto: Re: [Boost-users] Flyweight: wrapping shared_ptr
Le 7 oct. 2014 à 18:18, Joaquín Mª López Muñoz <joaquin@tid.es> a écrit :
An alternative would be to use some sort of cloning smart pointer to spare the reference count, but it's not clear to me this would be faster than shared_ptr:
* element exists: shared_ptr creation is slower than clone_ptr creation * element does not exist: shared_ptr creation and copy is probably faster than clone_ptr creation and cloning.
Yes, indeed.
A slightly smarter approach involves a cloning class that accepts a const Base& and does only clone on copy, thus avoiding dynamic memory allocation when the element already exists. This can be nicely wrapped up as follows: template<typename Base> class poly_holder { public: poly_holder(const Base& x):p(&x),dlt(false){} poly_holder(const poly_holder& x):p(x.p->clone()),dlt(true){} poly_holder& operator=(const poly_holder& x) { Base* q=x.p->clone(); if(dlt)delete p; p=q; dlt=true; return *this; } ~poly_holder(){if(dlt)delete p;} operator const Base&()const {return *p;} private: const Base* p; bool dlt; }; template<typename Base> class poly_flyweight:public boost::flyweight<poly_holder<Base>> { public: using super=boost::flyweight<poly_holder<Base>>; using super::super; const Base& operator*() const{return base();} const Base* operator->()const{return &base();} private: const Base& base()const{return this->get();} }; template<typename Base> std::size_t hash_value(const poly_flyweight<Base>& x) { return boost::hash<const Base*>()(&*x); } template<typename Base> bool operator==( const poly_flyweight<Base>& l,const poly_flyweight<Base>& r) { return static_cast<boost::flyweight<poly_holder<Base>>>(l)== static_cast<boost::flyweight<poly_holder<Base>>>(r); } template<typename Base> bool operator!=( const poly_flyweight<Base>& l,const poly_flyweight<Base>& r) { return !(l==r); } I've rewritten your test case to take advantage of this poly_flyweight thing, cleaning up some unnecessary hash and operator== overloads along the way and without resorting to the intermediate Exp_ type; see: http://coliru.stacked-crooked.com/a/76e87531b65d44f9
Is there anyway Flyweight could have supported the operator-> natively? It seems that with some SFINAE under the hood, it would work, don't you think?
Yep, that would be possible, the only reason I didn't do it is to not clutter the interface. Also, some std classes such as (C++17) std::optional implement operator* differently, without the extra dereference required by smart pointer semantics. Joaquín M López Muñoz Telefónica
Joaquin M Lopez Munoz <joaquin <at> tid.es> writes:
I've rewritten your test case to take advantage of this poly_flyweight thing, cleaning up some unnecessary hash and operator== overloads along the way and without resorting to the intermediate Exp_ type; see:
BTW, I forgot to add a virtual destructor to Base. Joaquín M López Muñoz Telefónica
Le 7 oct. 2014 à 20:41, Joaquin M Lopez Munoz <joaquin@tid.es> a écrit : Hi Joaquín,
A slightly smarter approach involves a cloning class that accepts a const Base& and does only clone on copy, thus avoiding dynamic memory allocation when the element already exists.
That's nice!
template<typename Base> bool operator==( const poly_flyweight<Base>& l,const poly_flyweight<Base>& r) { return static_cast<boost::flyweight<poly_holder<Base>>>(l)== static_cast<boost::flyweight<poly_holder<Base>>>(r); }
template<typename Base> bool operator!=( const poly_flyweight<Base>& l,const poly_flyweight<Base>& r) { return !(l==r); }
Yes, indeed, equality and hashing were uselessly deep in my example.
I've rewritten your test case to take advantage of this poly_flyweight thing, cleaning up some unnecessary hash and operator== overloads along the way and without resorting to the intermediate Exp_ type; see:
Thanks, I'll toy with it. Do you have any gut feeling about whether there should be only a single Exp-level hash-consing, or something with one flyweight per AST type (and conversions). I'm still considering both.
Is there anyway Flyweight could have supported the operator-> natively?
It seems that with some SFINAE under the hood, it would work, don't you think?
Yep, that would be possible, the only reason I didn't do it is to not clutter the interface. Also, some std classes such as (C++17) std::optional implement operator* differently, without the extra dereference required by smart pointer semantics.
You lost me here, I have no idea what special trick in std::optional would prevent Flyweight from completing its forwarding to such operators. I'll look for the proposal to try to understand the connection, thanks!
Akim Demaille <akim <at> lrde.epita.fr> writes:
Le 7 oct. 2014 à 20:41, Joaquin M Lopez Munoz <joaquin <at> tid.es> a écrit :
A slightly smarter approach involves a cloning class that accepts a const Base& and does only clone on copy, thus avoiding dynamic memory allocation when the element already exists.
That's nice!
Ah, you should add a move ctor and assignment operator for better performance. Revised example: http://coliru.stacked-crooked.com/a/2ab8d00da3f71c84
Do you have any gut feeling about whether there should be only a single Exp-level hash-consing, or something with one flyweight per AST type (and conversions). I'm still considering both.
Do you mean having using ExpBin=poly_flyweight<Bin>; using ExpNum=poly_flyweight<Num>; rather than a single using ExpBin=poly_flyweight<Bin>? I think this is very hard to manage: to begin with, Bin has two Exp members, with this type splitting it is not even clear how you would manage the different cases where the operands to Bin are compound expressions or Num's (and the combinations thereof).
Is there anyway Flyweight could have supported the operator-> natively? It seems that with some SFINAE under the hood, it would work, don't you think?
Yep, that would be possible, the only reason I didn't do it is to not clutter the interface. Also, some std classes such as (C++17) std::optional implement operator* differently, without the extra dereference required by smart pointer semantics.
You lost me here, I have no idea what special trick in std::optional would prevent Flyweight from completing its forwarding to such operators.
It is a basic difference in indirection handling. Let me explain: std::optional<T> defines operator-> as optional<T> --> const T* which, for the case of optional<ptr<T>> (ptr being some pointer-like type), yields optional<ptr<T>> --> const ptr<T>* For instance: optional<T*> --> const T** With the interface you propose for flyweight, we'd have operator-> for flyweight<ptr<T>> defined as optional<ptr<T>> --> const T* So, there is a choice between having -> behave with "pointer semantics" (std::optional) or "pass-thru" semantics, for want of a better name. If we choose pass-thru semantics, then we're ruling out the possibility of having operator-> in instantiations such as, say, flyweight<int>, where the flyweight'd element is not a (smart) pointer. If I were to decided, I'd go for pointer semantics, which is more generally applicable. On the other hand, you can have your own poly_flyweight<T> class like the one shown with pointer semantics (because the intermediate pointer indirection is hidden in the implementation). Not sure I made myself clear :-) Joaquín M López Muñoz Telefónica
Le 8 oct. 2014 à 14:32, Joaquin M Lopez Munoz <joaquin@tid.es> a écrit :
Akim Demaille <akim <at> lrde.epita.fr> writes:
Le 7 oct. 2014 à 20:41, Joaquin M Lopez Munoz <joaquin <at> tid.es> a écrit :
A slightly smarter approach involves a cloning class that accepts a const Base& and does only clone on copy, thus avoiding dynamic memory allocation when the element already exists.
That's nice!
Ah, you should add a move ctor and assignment operator for better performance. Revised example:
Great :) Finally, the free-lunch is not over, I should just let time pass and have my implementation improve all by itself :)
Do you have any gut feeling about whether there should be only a single Exp-level hash-consing, or something with one flyweight per AST type (and conversions). I'm still considering both.
Do you mean having
using ExpBin=poly_flyweight<Bin>; using ExpNum=poly_flyweight<Num>;
rather than a single using ExpBin=poly_flyweight<Bin>? I think this is very hard to manage: to begin with, Bin has two Exp members, with this type splitting it is not even clear how you would manage the different cases where the operands to Bin are compound expressions or Num's (and the combinations thereof).
I expect the flyweight implementation to support inheritance, just as std::shared_ptr<base> p = std::make_shared<derived>(); works. Which also means that I need variations around std::dynamic_pointer_cast etc., but it might be worth it. I have experimentations on this on my real project, but it does not work. I'll try to do that in the case of the simple toy hierarchy you and I used so far.
You lost me here, I have no idea what special trick in std::optional would prevent Flyweight from completing its forwarding to such operators.
It is a basic difference in indirection handling. Let me explain: std::optional<T> defines operator-> as
optional<T> --> const T*
which, for the case of optional<ptr<T>> (ptr being some pointer-like type), yields
optional<ptr<T>> --> const ptr<T>*
For instance:
optional<T*> --> const T**
With the interface you propose for flyweight, we'd have operator-> for flyweight<ptr<T>> defined as
optional<ptr<T>> --> const T*
So, there is a choice between having -> behave with "pointer semantics" (std::optional) or "pass-thru" semantics, for want of a better name.
Ouhh, I clearly prefer the pass-thru semantics here. The Flyweight is similar (to my eyes) to a proxy to an object that does all it can to have the rest of the code believe it is the real object. I'm very happy that I can enable/disable flyweight'ing via a simple typedef, and using a pointer-like semantics would break everything: the real object and the flyweight'ed one would have different interfaces.
If we choose pass-thru semantics, then we're ruling out the possibility of having operator-> in instantiations such as, say, flyweight<int>, where the flyweight'd element is not a (smart) pointer.
I'm not sure I understand what you mean here. I can't imagine what 'flyweight<int>(42)->...' would mean. I'd sfinae it out of the picture.
If I were to decided, I'd go for pointer semantics, which is more generally applicable. On the other hand, you can have your own poly_flyweight<T> class like the one shown with pointer semantics (because the intermediate pointer indirection is hidden in the implementation). Not sure I made myself clear :-)
Actually it's precisely because it forwards the operator-> call to the wrapped that I see this as pass-thru. The operator-> and operator* behaves as if I were talking to the shared_ptr, not to the flyweight. But maybe I misunderstood what you meant here.
Akim Demaille <akim <at> lrde.epita.fr> writes:
Le 8 oct. 2014 à 14:32, Joaquin M Lopez Munoz <joaquin <at> tid.es> a écrit :
Ah, you should add a move ctor and assignment operator for better performance. Revised example:
Great :) Finally, the free-lunch is not over, I should just let time pass and have my implementation improve all by itself :)
Anyway, please profile against shared_ptr, I'm not claiming this is necessarily faster.
Do you mean having
using ExpBin=poly_flyweight<Bin>; using ExpNum=poly_flyweight<Num>;
rather than a single using ExpBin=poly_flyweight<Bin>? I think this is very hard to manage: to begin with, Bin has two Exp members, with this type splitting it is not even clear how you would manage the different cases where the operands to Bin are compound expressions or Num's (and the combinations thereof).
I expect the flyweight implementation to support inheritance, just as
std::shared_ptr<base> p = std::make_shared<derived>();
This is alas not the case for flyweight<base>/flyweight<derived>. You can simulate something like that behavior in poly_flyweight: template<typename Derived> class poly_flyweight { ... operator flyweight<Base>()const; ... }; but invoking this operator for a poly_flyweight<Derived> object x would imply creating a clone of x.get() (upcasted to const Base*) in poly_flyweight<Base> internal factory, which is a waste of space and brings you to the original situation where all objects are stored in the same factory. A less ambitious approach is template<typename Derived,typename Base> class derived_poly_flyweight:poly_flyweight<Base> { ... const Derived& operator()const; // similar for operator* and operator-> ... }; which gives you direct conversion to const Derived&, but still stores everything in the same factory.
So, there is a choice between having -> behave with "pointer semantics" (std::optional) or "pass-thru" semantics, for want of a better name.
Ouhh, I clearly prefer the pass-thru semantics here. The Flyweight is similar (to my eyes) to a proxy to an object that does all it can to have the rest of the code believe it is the real object. I'm very happy that I can enable/disable flyweight'ing via a simple typedef, and using a pointer-like semantics would break everything: the real object and the flyweight'ed one would have different interfaces.
I understand your rationale, but pass-thru semantics is only applicable to pointer-like elements.
If we choose pass-thru semantics, then we're ruling out the possibility of having operator-> in instantiations such as, say, flyweight<int>, where the flyweight'd element is not a (smart) pointer.
I'm not sure I understand what you mean here. I can't imagine what 'flyweight<int>(42)->...' would mean. I'd sfinae it out of the picture.
int is a poorly chosen example. Take instead struct foo { int x; }; With pointer-like semantics, you have flyweight<foo> f; flyweight<foo*> g; f->x=0; // OK g->x=0; // error with pass-thru semantics, it is the other way around: f->x=0; // error g->x=0; // OK The case with poly_flyweight is different. As poly_flyweight<Base> is expected to act as a replacement for const Base*, pass-thru semantics makes a lot of sense. Joaquín M López Muñoz Telefónica
Joaquin M Lopez Munoz <joaquin <at> tid.es> writes:
With pointer-like semantics, you have
flyweight<foo> f; flyweight<foo*> g;
f->x=0; // OK g->x=0; // error
This is again a bad example because stored elements are treated as const :-) Consider instead std::cout<<f->x; std::cout<<g->x; for instance. You get the idea, anyway. Joaquín M López Muñoz Telefónica
Le 8 oct. 2014 à 15:41, Joaquin M Lopez Munoz <joaquin@tid.es> a écrit :
Joaquin M Lopez Munoz <joaquin <at> tid.es> writes:
With pointer-like semantics, you have
flyweight<foo> f; flyweight<foo*> g;
f->x=0; // OK g->x=0; // error
This is again a bad example because stored elements are treated as const :-) Consider instead
std::cout<<f->x; std::cout<<g->x;
for instance. You get the idea, anyway.
Yes, I think this time I see what you mean. You're proposing boost::flyweight<std::string> s("foo"); std::cerr << s->size() << std::endl; to make sense, right?
Akim Demaille <akim <at> lrde.epita.fr> writes:
Le 8 oct. 2014 à 15:41, Joaquin M Lopez Munoz <joaquin <at> tid.es> a écrit :
Joaquin M Lopez Munoz <joaquin <at> tid.es> writes:
With pointer-like semantics, you have
flyweight<foo> f; flyweight<foo*> g;
f->x=0; // OK g->x=0; // error
[...]
std::cout<<f->x; std::cout<<g->x;
for instance. You get the idea, anyway.
Yes, I think this time I see what you mean. You're proposing
boost::flyweight<std::string> s("foo"); std::cerr << s->size() << std::endl;
to make sense, right?
Correct. Pointer semantics and pass-thru semantics cannot coexist in the same interface (not that I'm currently considering supporting either, anyway). For your case this is not a problemm, since you'll be using some incarnation of your custom poly_flyweight, which you can endow with whatever -> semantics you like (pass-thru, I presume). Joaquín M López Muñoz Telefónica
Le 8 oct. 2014 à 15:33, Joaquin M Lopez Munoz <joaquin@tid.es> a écrit :
I expect the flyweight implementation to support inheritance, just as
std::shared_ptr<base> p = std::make_shared<derived>();
This is alas not the case for flyweight<base>/flyweight<derived>. You can simulate something like that behavior in poly_flyweight:
template<typename Derived> class poly_flyweight { ... operator flyweight<Base>()const; ... };
but invoking this operator for a poly_flyweight<Derived> object x would imply creating a clone of x.get() (upcasted to const Base*) in poly_flyweight<Base> internal factory, which is a waste of space and brings you to the original situation where all objects are stored in the same factory.
I used something more agressive: template <typename T> class poly_flyweight : public boost::flyweight<poly_holder<T>> { public: using super = boost::flyweight<poly_holder<T>>; using super::super; /// Conversion to superclass. template <typename U> operator const poly_flyweight<U>&() const { const void* me = this; return *static_cast<const poly_flyweight<U>*>(me); } const T& operator*() const { return base(); } const T* operator->() const { return &base(); } private: const T& base()const { return this->get(); } }; of course I need to enable this conversion only when one can convert from T to U. However, while this adresses: NumFW a = num(1); Exp b = a; it does not work for: Exp c(a); even if I try to add a constructor to the poly_flyweight: template <typename U> explicit poly_flyweight(const poly_flyweight<U>& x) : poly_flyweight(static_cast<const poly_flyweight&>(x).base()) {} I have to make it explicit to avoid ambiguities, but then my ctor is never ever called. I suspect that there is a perfect forwarding constructor inside boost::flyweight that is preferred by the compiler. I have quite a headhack now, I gotta stop :) Again, thanks a lot Joaquín. (Actually it looks like what I did is not even portable: my clang 3.5 is happy with it, but not Coliru's gcc http://coliru.stacked-crooked.com/a/0cfe34a8ba934fea)
Akim Demaille <akim <at> lrde.epita.fr> writes:
I used something more agressive:
[...[
/// Conversion to superclass. template <typename U> operator const poly_flyweight<U>&() const { const void* me = this; return *static_cast<const poly_flyweight<U>*>(me); }
Wow, this is totally verboten. The following problems lurk: 1. poly_flyweight<U> need not be layout compatible with poly_flyweight<T>. 2. Even if #1 holds, a pointer to Base need not be numerically equivalent to a pointer to Derived (in the presence of multiple inheritance, for instance.) 3. Even if #2 holds, the thing would work by pure chance (basically, because a "casted" poly_flyweight<Base> happens never to touch the internal factory of poly_flyweight<Derived>, which in principle a conformant implementation is allowed to do). All in all, the thing could potentially work, but why would that be preferred over derived_poly_flyweight<Derived,Base>? Joaquin M López Muñoz Telefónica
Joaquin M Lopez Munoz <joaquin <at> tid.es> writes:
Wow, this is totally verboten. The following problems lurk:
[...]
3. Even if #2 holds, the thing would work by pure chance (basically, because a "casted" poly_flyweight<Base> happens never to touch the internal factory of poly_flyweight<Derived>, which in principle a conformant implementation is allowed to do).
In fact, the invalid scenario can happen at flyweight destruction time. See the program crash at: http://coliru.stacked-crooked.com/a/3180b67524a49b75 Joaquín M López Muñoz Telefónica
Hi Joaquin! Le 8 oct. 2014 à 20:57, Joaquin M Lopez Munoz <joaquin@tid.es> a écrit :
Akim Demaille <akim <at> lrde.epita.fr> writes:
/// Conversion to superclass. template <typename U> operator const poly_flyweight<U>&() const { const void* me = this; return *static_cast<const poly_flyweight<U>*>(me); }
Wow, this is totally verboten. The following problems lurk[...]
Thanks for the details, and the real case. Thanks also for prompting me to dig deeper into the derived_poly_flyweight. I think it now does what I want, if you're curious: http://coliru.stacked-crooked.com/a/cacc6107cf22c2e6 I made several changes: - I want to be able to dynamic_pointer_cast, so I added such a function - but so I also need to support null pointers, so I have added support for nullptr and operator bool. - because of this, I had to write specific hash_value and operator== for poly_holder, otherwise the pointer gets dereferenced (BTW, I think it's name is confusing, as in Boost.Flyweight "holder" denotes something quite different). - FWIW, your operator== for poly_flyweight was uselessly deep, comparing the addresses should suffice (actually, shouldn't Boost.Flyweight provide these operators itself? Is there any possibility that something else would be wanted? I'm also thinking about hashing here). Here's the kind of things that works: Bin e1 = bin('*', bin('+', num(1), num(1)), bin('+', num(1), num(1))); ECHO(e1); ECHO(e1->l); Exp e2 = e1; Exp e3(e1); ECHO(e3); ECHO(dynamic_pointer_cast<Bin_impl>(e3)->op); ECHO(dynamic_pointer_cast<Num_impl>(e3) ? "FAIL" : "pass"); Again, many thanks for your help.
Akim Demaille <akim <at> lrde.epita.fr> writes:
I think it now does what I want, if you're curious:
Looks neat!
I made several changes:
[...]
- but so I also need to support null pointers, so I have added support for nullptr and operator bool.
I think you forgot to add that support in some of poly_older ctors and assignment operators: poly_holder(const poly_holder& x); poly_holder& operator=(const poly_holder& x); poly_holder& operator=(poly_holder&& x);
- FWIW, your operator== for poly_flyweight was uselessly deep, comparing the addresses should suffice
You're right, your version is the right one.
(actually, shouldn't Boost.Flyweight provide these operators itself? Is there any possibility that something else would be wanted? I'm also thinking about hashing here).
Currently, the following overloads are provided: ==,<,!=,>,>=,<=,swap,<< hash was not provided for no particular reason, but I'll consider adding it (both for boost::hash and std::hash). As for what these overloads are not serving your poly_flyweight, the problem is that in poly_flyweight<Base> f1,f2; f1==f2; // ambiguity the == expression is probing many implicit conversions (to poly_flyweight<Base>, to poly_holder<Base>, to Base) resulting in ambiguities: this is why you need to suply your own == etc. for poly_flyweight. Good luck with you project. Joaquín M López Muñoz Telefónica
Le 9 oct. 2014 à 12:35, Joaquin M Lopez Munoz <joaquin@tid.es> a écrit :
I think you forgot to add that support in some of poly_older ctors and assignment operators:
poly_holder(const poly_holder& x); poly_holder& operator=(const poly_holder& x); poly_holder& operator=(poly_holder&& x);
Yes, thanks. I wanted to try another approach: one factory per node type. Here is a version that seems to work, but does not use Boost.Flyweight at all. Instead each node type maintains the factory as a plain std::map<tuple<Arguments>, weak_ptr<Value>>. This is a weak_ptr otherwise if I store shared_ptr, then of course the values are kept alive by the factory itself. This is the result: http://coliru.stacked-crooked.com/a/28bb07a0b0e59fbb For instance Bin reads: struct Bin_impl; using Bin = std::shared_ptr<Bin_impl>; struct Bin_impl: Exp_impl { // cannot make it private: we need friendship with // std::make_shared<Bin_impl>'s internal details, which seems // impossible to spell out portably. Bin_impl(char o, Exp lhs, Exp rhs) : op(o) , l(lhs) , r(rhs) {} public: ~Bin_impl() {} static Bin make(char o, Exp lhs, Exp rhs) { static std::map<std::tuple<char, Exp, Exp>, std::weak_ptr<Bin_impl>> map_; auto k = std::make_tuple(o, lhs, rhs); auto i = map_.find(k); if (i == end(map_) || i->second.expired()) { auto res = std::make_shared<Bin_impl>(o, lhs, rhs); map_.emplace(k, res); return res; } else return Bin(i->second); } ... Which is ok. However, I tried to do the same thing this time on top of Flyweight, using key_value, but failed miserably: the result is not made unique: http://coliru.stacked-crooked.com/a/2b768fa26574adea I tried this way: struct Num_impl; using Num = std::shared_ptr<const Num_impl>; std::ostream& operator<<(std::ostream& o, const Num_impl& b); struct Num_impl { //private: Num_impl(int v) : val(v) { std::cerr << '!' << *this << '\n'; } ~Num_impl() { std::cerr << "~" << *this << '\n'; } struct make_num : boost::noncopyable { make_num(int n) : res(std::make_shared<Num_impl>(n)) {} Num res; operator Num() const { return res; } }; public: static Num make(int v) { using flyweight = boost::flyweight<boost::flyweights::key_value<int,make_num>>; return flyweight(v).get(); } std::ostream& print(std::ostream& o) const { return o << val; } int val = 0; }; I suppose that it's a bad idea for Num::make to return the result of the call to get(), as this prevents the flyweight from tracking the values. Yet the destructor was not called, so I don't understand why Flyweight fails to see the duplicate 42.
Akim Demaille <akim <at> lrde.epita.fr> writes:
Which is ok. However, I tried to do the same thing this time on top of Flyweight, using key_value, but failed miserably: the result is not made unique:
[...]
struct make_num : boost::noncopyable { make_num(int n) : res(std::make_shared<Num_impl>(n)) {}
Num res; operator Num() const { return res; } };
public: static Num make(int v) { using flyweight = boost::flyweight<boost::flyweights::key_value<int,make_num>>; return flyweight(v).get(); }
This is what's hapenning: 1. make(v) creates a *temporary* flyweight(v), which contains a make_num struct containing a std::shared_ptr<Num_impl>. 2. make(v) returns a copy of the shaered_ptr, which gets then a ref count of 2. 3. when make(v) exists, the temporary flyweight is destroyed, and as *there's no other flyweight with the same value*, the associated make_num struct gets erased from the flyweight internal factory. 4. Erasing the make_num results in its shared_ptr member being destroyed, hence the referenced to Num_impl gets a ref count of 1: it is not destroyed, this is why ~Num_impl is not invoked. 5. Next time you call make(v), there's no longer an equivalent value in the factoty, 1-4 are repeated and make(v) returns a shared_ptr to a *different* Num_impl. In short: what keeps a value around in a flyweight factory is the *flyweight* ref count, *not* the *shared_ptr* ref count of one of its members. Why do you want to have a factory per node type? Hash containers' performance is basically independent of the number of elements being contained, so you won't get any significant speedup by separating types --what's more, you can even get a performance reduction due to CPU cache misses, since having more hashed containers around places more bucket arrays in the program's working memory. Joaquín M López Muñoz Telefónica
Le 9 oct. 2014 à 22:20, Joaquin M Lopez Munoz <joaquin@tid.es> a écrit :
This is what's hapenning:
1. make(v) creates a *temporary* flyweight(v), which contains a make_num struct containing a std::shared_ptr<Num_impl>.
Ah, yes, of course. Thanks. This is fixed now.
Why do you want to have a factory per node type?
To be able to try it and to bench it. I'd like to be sure about the choice to make wrt the overhaul of my ASTs, and the options I have so far seem to be: a. a single factory based on Flyweight, and use derived_poly_flyweight b. one factory made by hand on top of map or unordered_map c. likewise, but with Flyweight and key_value
Hash containers' performance is basically independent of the number of elements being contained, so you won't get any significant speedup by separating types --what's more, you can even get a performance reduction due to CPU cache misses, since having more hashed containers around places more bucket arrays in the program's working memory.
Well, this is not so clear to me, as the expressions I store have often a strong regularity, with many elements of the same kind in a row. Besides, I appreciate that I can get rid of one level of indirection, as the factories now store genuine objects, not pointers to them. And I expect this to have quite a positive impact on the performances. b features this, and has the advantage that there are fewer types to deal with in the library: it's just shared_ptr and that's something people know. But actually b does not work properly yet, and I probably have to deal with ref-counting myself, because the version which is in the store should not count. There are contributors and users of this library, so I am also concerned with the learning curve for people who would want to use it. Yet speed is a primary concern. Since eventually we will certainly go parallel, I also appreciate Flyweight's safety. So really, I'd like to have toy implementations of a, b, and c, and them decide what to do (well, discuss with the other members and see what conclusion emerges). I feel that Boost.Flyweight has everything I need: it keeps the right ref counter, can support key-value map, it is safe, well documented etc. Yet I have to find a means to get polymorphism back in business.
Akim Demaille <akim <at> lrde.epita.fr> writes:
Le 9 oct. 2014 à 22:20, Joaquin M Lopez Munoz <joaquin <at> tid.es> a
écrit :
Why do you want to have a factory per node type?
To be able to try it and to bench it. I'd like to be sure about the choice to make wrt the overhaul of my ASTs, and the options I have so far seem to be:
a. a single factory based on Flyweight, and use derived_poly_flyweight
b. one factory made by hand on top of map or unordered_map
c. likewise, but with Flyweight and key_value
As for c, you can have something more or less along that line by using your latest attempt (http://coliru.stacked-crooked.com/a/2b768fa26574adea ), declaring the flyweights with the no_tracking policy and having them store a weak_ptr (which boild down to your b hand-made scenario.)
Hash containers' performance is basically independent of the number of elements being contained [...]
Well, this is not so clear to me, as the expressions I store have often a strong regularity, with many elements of the same kind in a row.
Of course, benchmarking is the best way to go. I'd be interested in knowing your outcomes. Joaquín M López Muñoz Telefónica
On 09-10-2014 11:50, Akim Demaille wrote:
I think it now does what I want, if you're curious:
Just took a quick look. I would prefer if you used scoped_ptr inside poly_holder. regards -Thorsten
Thorsten Ottosen <tottosen <at> dezide.com> writes:
On 09-10-2014 11:50, Akim Demaille wrote:
I think it now does what I want, if you're curious:
Just took a quick look. I would prefer if you used scoped_ptr inside poly_holder.
Hi Thorsten, poly_holder has a very different semantics than scoped_ptr's: 1. Constructing from a const Base& does not involve heap allocation. 2. Copying involves either heap allocation and cloning if the copied elements didn't allocate, or ownership transfer otherwise. Joaquín M López Muñoz Telefónica
On 09-10-2014 22:23, Joaquin M Lopez Munoz wrote:
Thorsten Ottosen <tottosen <at> dezide.com> writes:
Just took a quick look. I would prefer if you used scoped_ptr inside poly_holder.
Hi Thorsten,
poly_holder has a very different semantics than scoped_ptr's:
1. Constructing from a const Base& does not involve heap allocation. 2. Copying involves either heap allocation and cloning if the copied elements didn't allocate, or ownership transfer otherwise.
Ok, so pick any smart pointer that allows you to do what you want, e.g. unique_ptr with a custom deleter. I'm just looking at the code and thinking there's some potential for exception-safety issues which would probably not be the case if the pointer was stored differently. Also, I can't figure our the invariant. The pointer can be null, in which case a call to delete is expensive (unique_ptr would handle this automatically). regards -Thorsten
participants (3)
-
Akim Demaille -
Joaquin M Lopez Munoz -
Thorsten Ottosen