[review] Review of Flyweight library started January 21 and still running!

Hi all, The formal review of Flyweight library started January 21 and will end January 30, we have a few nice reviews, but I'm sure that those who haven't reviewed it are also interested in the library ;-). The library solves quite a common programming pattern, is small and very well documented. So don't be afraid to try it! Information for reviewers: *Description:* Flyweights are small-sized handle classes granting constant access to shared common data, thus allowing for the management of large amounts of entities within reasonable memory limits. Boost.Flyweight makes it easy to use this common programming idiom by providing the class template flyweight<T>, which acts as a drop-in replacement for const T. *Online docs:* http://tinyurl.com/2sstyr http://svn.boost.org/svn/boost/sandbox/flyweight/libs/flyweight/index.html *Download:* http://tinyurl.com/hrdm6 http://www.boost-consulting.com/vault/index.php?&direction=0&order=&directory=Patterns *Notes:* 1) We've seen some suggestions in the mailing list for Flyweight. Joaquín has nicely explained a couple of issues that we'd like to address/discuss in the review: http://tinyurl.com/33ghtf http://svn.boost.org/svn/boost/sandbox/flyweight/libs/flyweight/doc/review_n... 2) Flyweight needs Boost 1.35 elements because the library depends on libraries like Interprocess for some features/tests. Since SVN snapshot tarballs seem to be missing these days, those who want to try flyweight can download a working SVN-HEAD snapshot here: http://igaztanaga.drivehq.com/boost_trunk.tar.bz2 3) Serialization tests won't work. This feature is expected to work when some new features (discussed in the mailing list between Joaquín and Robert Ramey) are added in Boost.Serialization. Those are expected for Boost 1.36. What to include in Review Comments ================================== Your comments may be brief or lengthy, but basically the Review Manager needs your evaluation of the library. If you identify problems along the way, please note if they are minor, serious, or showstoppers. Here are some questions you might want to answer in your review: * 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? And finally, every review should answer this question: * Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. Ion Gaztañaga - Review Manager -

Hi all, equality semantic bool b=(&(fw1.get())==&(fw2.get())); versus bool b=((fw1.get())==(fw2.get())); Why not delegate this responsability to the factory, the factory knows better when the two handles are equal, isn't it? friend bool operator==(const flyweight& x,const flyweight& y) { return core:are_equals(x.h==y.h); } The factories provided by the library could define this are_equals function as return (&x==&y); Regards--------------------------- Vicente Juan Botet Escriba

----- Mensaje original ----- De: "vicente.botet" <vicente.botet@wanadoo.fr> Fecha: Domingo, Enero 27, 2008 7:10 pm Asunto: Re: [boost] [review] Review of Flyweight library started January 21and still running!: equality semantic Para: boost@lists.boost.org
Hi all, equality semantic bool b=(&(fw1.get())==&(fw2.get())); versus bool b=((fw1.get())==(fw2.get())); Why not delegate this responsability to the factory, the factory knows better when the two handles are equal, isn't it?
friend bool operator==(const flyweight& x,const flyweight& y) { return core:are_equals(x.h==y.h); }
The factories provided by the library could define this are_equals function as return (&x==&y); Regards---------------------------
Umm, this approach is interesting! Better yet, instead of relying on an explicit are_equals function, we can just require that Factory::handle_type is EqualityComparable and use that: friend bool operator==(const flyweight& x,const flyweight& y) { return x.h==y.h; } As a matter of fact, all the current factories have EqualityComparable handles that precisely implement reference equality semantics, so it looks all very natural. This approach would solve the second problem of the list I presented at the review notes: * The resulting equality semantics might not coincide with that of the underlying T; the possibility of this hapenning and making a real difference is probably small, though. * The assumption that equal flyweight objects always share their value interfere with potential extensions to the library, like the no-factory proposal by Throsten Ottosen. but would not address the first point. Thanks for your suggestion, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

Thanks Joaquin for your response. JOAQUIN LOPEZ MUÑOZ wrote
Umm, this approach is interesting! Better yet, instead of relying on an explicit are_equals function, we can just require that Factory::handle_type is EqualityComparable and use that:
friend bool operator==(const flyweight& x,const flyweight& y) { return x.h==y.h; }
As a matter of fact, all the current factories have EqualityComparable handles that precisely implement reference equality semantics, so it looks all very natural. This approach would solve the second problem of the list I presented at the review notes:
* The resulting equality semantics might not coincide with that of the underlying T; the possibility of this hapenning and making a real difference is probably small, though. * The assumption that equal flyweight objects always share their value interfere with potential extensions to the library, like the no-factory proposal by Throsten Ottosen.
This seams OK to me, finally it will be the responsability of the factory to define the equality operator for its handle_type.
but would not address the first point.
I think that I don't really understant the first problem: if we have T t1, t2 such t1==T2 it may happens that handle_type of t1 != handle_type of t2 Please could you give us un example on which the first problem is present? Regards --------------------------- Vicente Juan Botet Escriba

Thanks Joaquin for your response.
JOAQUIN LOPEZ MUÑOZ wrote
Umm, this approach is interesting! Better yet, instead of relying on an explicit are_equals function, we can just require
----- Mensaje original ----- De: "vicente.botet" <vicente.botet@wanadoo.fr> Fecha: Domingo, Enero 27, 2008 10:17 pm Asunto: Re: [boost] [review] Review of Flyweight library started January 21 and still running!: equality semantic Para: boost@lists.boost.org that
Factory::handle_type is EqualityComparable and use that:
friend bool operator==(const flyweight& x,const flyweight& y) { return x.h==y.h; }
As a matter of fact, all the current factories have EqualityComparable handles that precisely implement reference equality semantics, so it looks all very natural. This approach would solve the second problem of the list I presented at the review notes:
* The resulting equality semantics might not coincide with that of the underlying T; the possibility of this hapenning and making a real difference is probably small, though. * The assumption that equal flyweight objects always share their value interfere with potential extensions to the library, like the no-factory proposal by Throsten Ottosen.
This seams OK to me, finally it will be the responsability of the factory to define the equality operator for its handle_type.
but would not address the first point.
I think that I don't really understant the first problem: if we have T t1, t2 such t1==T2 it may happens that handle_type of t1 != handle_type of t2
Please could you give us un example on which the first problem is present? Regards
The situation would be one in which the equivalence criteria used by the flyweight factory would not be the same as induced by T::operator==, that is, if two flyweight objects fw1, fw2 are such that fw1!=fw2 yet fw1.get()==fw2.get() (the converse, i.e. fw1==fw2 and fw1.get()!=fw2.get(), cannot happen). The following is an admittedly artificial example: class istring:public std::string { public: istring(const char* p):std::string(p){} friend inline bool operator==(const istring& x,const istring& y) { return stricmp(x.c_str(),y.c_str())==0; // case insensitive } }; typedef flyweight<istring,set_factory<> > fw_t; int main() { fw_t fw1("hello"),fw2("HELLO"); assert(fw1!=fw2); assert(fw1.get()==fw2.get()); } Upon seeing this example, I'm growing more suspicuous that maybe I'm being too precautious: situations like this are extremely unlikely. Maybe the design is good as it is, and equality of flyweights should just be based on &-equality. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

JOAQUIN LOPEZ MU?Z wrote
* The resulting equality semantics might not coincide with that of the underlying T; the possibility of this hapenning and making a real difference is probably small, though.
Please could you give us un example on which the first problem is present?
The situation would be one in which the equivalence criteria used by the flyweight factory would not be the same as induced by T::operator==, that is, if two flyweight objects fw1, fw2 are such that fw1!=fw2 yet fw1.get()==fw2.get() (the converse, i.e. fw1==fw2 and fw1.get()!=fw2.get(), cannot happen). The following is an admittedly artificial example:
class istring:public std::string { public: istring(const char* p):std::string(p){} friend inline bool operator==(const istring& x,const istring& y) { return stricmp(x.c_str(),y.c_str())==0; // case insensitive } };
typedef flyweight<istring,set_factory<> > fw_t;
int main() { fw_t fw1("hello"),fw2("HELLO"); assert(fw1!=fw2); assert(fw1.get()==fw2.get()); }
Upon seeing this example, I'm growing more suspicuous that maybe I'm being too precautious: situations like this are extremely unlikely. Maybe the design is good as it is, and equality of flyweights should just be based on &-equality.
If I understand the class istring will work well with a default hashed_factory because the default factory use the == operator to state when the value can be shared. This will not be the case if another Predicate is used as parameter to the hashed_factory. The problem with the set_factory for this class is that istring("hello") == istring(HELLO) and istring("hello") < istring(HELLO). You are right this is a quite extrange. So if a value type T satisfy for all T t1, t2 / t1==t2 => !((t1<t2) || (t2<t1)) then we can define the flyweight equality as follows: two flyweight objects can be considered equal if they share their value but if this is not satisfied the flyweight equality must resort to the underlying values. The problem is that this can not be cheched, so why not rely on some traits class and let the user state what he/she prefers. // by default considered equal if they share their value template <typename T> struct flyweight_equals_if_shared_value : boost::mpl::true_{}; // resort to the underlying values. template <> struct flyweight_equals_if_shared_value<istring> : boost::mpl::false_{}; Can this be used to solve the equality semantic problems? Regrads --------------------------- Vicente Juan Botet Escriba

----- Mensaje original ----- De: "vicente.botet" <vicente.botet@wanadoo.fr> Fecha: Lunes, Enero 28, 2008 8:48 pm Asunto: Re: [boost] [review] Review of Flyweight library started January 21 and still running!: equality semantic Para: boost@lists.boost.org
JOAQUIN LOPEZ MU?Z wrote
The situation would be one in which the equivalence criteria used by the flyweight factory would not be the same as induced by T::operator==, that is, if two flyweight objects fw1,
fw2
are such that fw1!=fw2 yet fw1.get()==fw2.get() (the converse, i.e. fw1==fw2 and fw1.get()!=fw2.get(), cannot happen). The following is an admittedly artificial example:
class istring:public std::string { public: istring(const char* p):std::string(p){} friend inline bool operator==( const istring& x,const istring& y) { return stricmp(x.c_str(),y.c_str())==0; // case insensitive } };
typedef flyweight<istring,set_factory<> > fw_t;
int main() { fw_t fw1("hello"),fw2("HELLO"); assert(fw1!=fw2); assert(fw1.get()==fw2.get()); }
Upon seeing this example, I'm growing more suspicuous that maybe I'm being too precautious: situations like this are extremely unlikely. Maybe the design is good as it is, and equality of flyweights should just be based on &-equality.
[...]
The problem with the set_factory for this class is that istring("hello") == istring(HELLO) and istring("hello") < istring(HELLO).
Correct.
[...] so why not rely on some traits class and let the user state what he/she prefers.
// by default considered equal if they share their value template <typename T> struct flyweight_equals_if_shared_value : boost::mpl::true_{};
// resort to the underlying values. template <> struct flyweight_equals_if_shared_value<istring> : boost::mpl::false_{}; Can this be used to solve the equality semantic problems?
Yep, this is a variant of another reviewer's proposal of having this bit of info as an extra template parameter. Either way is realizable. What I'm not so sure, after discussing the issue with you, is if we really should worry about this rather pathological situation: after all, if a user ever faces it, she just can override oeprator== with non-& semantics and be done. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

JOAQUIN LOPEZ MU?Z wrote
[...] so why not rely on some traits class and let the user state what he/she prefers.
// by default considered equal if they share their value template <typename T> struct flyweight_equals_if_shared_value : boost::mpl::true_{};
// resort to the underlying values. template <> struct flyweight_equals_if_shared_value<istring> : boost::mpl::false_{}; Can this be used to solve the equality semantic problems?
Yep, this is a variant of another reviewer's proposal of having this bit of info as an extra template parameter. Either way is realizable. What I'm not so sure, after discussing the issue with you, is if we really should worry about this rather pathological situation: after all, if a user ever faces it, she just can override oeprator== with non-& semantics and be done.
I think that the problem is related to the underlying type on the context of a flyweight. This approach do not needs to pass an extra parameter each time the type is used as a flyweight. Any way is up to you to decide whether your library takes this cases in account or not. If you decide to not manage these pathological cases, un example will help the user to understand the problem. Regards _______________________________________________ Vicente Juan Botet Escriba

Hi again I surelly mis something from the standard C++. I'm asking what happens if we declare static flyweight as static flyweight<std::string> static_var1("safe_or_unsafe1"); static flyweight<std::string>::initializer fwinit; static flyweight<std::string> static_var2("safe_or_unsafe2"); Which is the destruction order between static_var1, static_var2 and the static holder of flyweight<std::string>? --------------------------- Vicente Juan Botet Escriba

----- Mensaje original ----- De: "vicente.botet" <vicente.botet@wanadoo.fr> Fecha: Domingo, Enero 27, 2008 10:42 pm Asunto: Re: [boost] [review] Review of Flyweight library started January21 and still running!: static flyweights Para: boost@lists.boost.org
Hi again
I surelly mis something from the standard C++. I'm asking what happens if we declare static flyweight as
static flyweight<std::string> static_var1("safe_or_unsafe1"); static flyweight<std::string>::initializer fwinit; static flyweight<std::string> static_var2("safe_or_unsafe2");
Which is the destruction order between static_var1, static_var2 and the static holder of flyweight<std::string>?
The destruction order is the correct one: static_var2 static_var1 holder I guess you're being misled by the definition of the initializer fwinit: Its presence forces holder initialization *unless* this initialization has already taken place, which is the case here, due to the previous definition of static_var1. So, your code is entirely equivalent to static flyweight<std::string> static_var1("safe_or_unsafe1"); static flyweight<std::string> static_var2("safe_or_unsafe2"); Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

JOAQUIN LOPEZ MUÑOZ wrotes
The destruction order is the correct one:
static_var2 static_var1 holder
I guess you're being misled by the definition of the initializer fwinit: Its presence forces holder initialization *unless* this initialization has already taken place, which is the case here, due to the previous definition of static_var1. So, your code is entirely equivalent to
static flyweight<std::string> static_var1("safe_or_unsafe1"); static flyweight<std::string> static_var2("safe_or_unsafe2");
You are right. The destruction order is correct. I have associated the static initialization of the holder and the fwinit variable and have forgotten that the static_var1 initialization forces the static initialization of the holder. Sorry --------------------------- Vicente Juan Botet Escriba

Hi all, I don't see any comment during the review respect to the "Alternative/simpler configuration interface" originated by Peter Dimov. http://svn.boost.org/svn/boost/sandbox/flyweight/libs/flyweight/doc/review_n... Joaquin, you write in this section "An aim of Boost.Flyweight is to provide maximum customizability so as to meet everybody's needs from basic users to programmers with very specific requirements" I think that the proposal of Peter is more open, because there are less constraints in the template parameter. But even in this case the user needs factories, and the policy problem will now be reported on the factory class. The real problem is if the current interface reponds to the goal of the library. I'm not sure that the current policies are all orthogonal, and complete respect to the domain, I need more time to understand how all these policies interact between. For example, can the current design manage with interprocess flyweights? I'm thinking about the possibility to have flyweights fields in a multi_index stored on shared memeory, for example.Which policies will we reused and which ones added? If this is possible and natural, do you think that this could be included in the list of future work? Maybe this should be managed orthogonally but, can the current design manage with persistent flyweights? Which policies will we reused and which ones added? If this is possible and natural, do you think that this could be included in the list of future work? Joaquin I understand your concern, if we need to change the core then we recover only some lines of code of the flyweight class. So we can ask ourselfs why not to define a new class? Any way, let me purpose you an alternative witch do not suffer of "demands a slightly less terse syntax" // default configuration typedef flyweight<T> fw_t1; // change core implementation typedef flyweight<T, core<shared_ptr_core> > fw_t2; // or even with the deduced parameters, change core implementation typedef flyweight<T, shared_ptr_core > fw_t2; // change some aspect of the default core as now typedef flyweight<T, no_tracking > fw_t3; This will complicate only the flyweight class which needs to take care of the exclusion of the parameters. And of course this will need a description of what is expected from a core. I don't know if the Boost.Parameter library takes care of the exclusion aspect? What do you think? --------------------------- Vicente Juan Botet Escriba

"vicente.botet" ha escrito:
Hi all,
I don't see any comment during the review respect to the "Alternative/simpler configuration interface" originated by Peter Dimov. http://svn.boost.org/svn/boost/sandbox/flyweight/libs/flyweight/doc/review_n...
Joaquin, you write in this section "An aim of Boost.Flyweight is to provide maximum customizability so as to meet everybody's needs from basic users to programmers with very specific requirements"
I think that the proposal of Peter is more open, because there are less constraints in the template parameter. But even in this case the user needs factories, and the policy problem will now be reported on the factory class.
Correct. More open here does not necessarily mean more powerful --we simply charge the user with more work to do. The question is whether the current policy decomposition is good enough so as to not need a blanket variation such as Peter's proposal.
The real problem is if the current interface reponds to the goal of the library. I'm not sure that the current policies are all orthogonal, and complete respect to the domain, I need more time to understand how all these policies interact between.
For example, can the current design manage with interprocess flyweights? I'm thinking about the possibility to have flyweights fields in a multi_index stored on shared memeory, for example.Which policies will we reused and which ones added? If this is possible and natural, do you think that this could be included in the list of future work?
This can be done by providing an interprocess-compatible holder, factory and locking policy. Tracking would not be affected. Instead of a custom factory, we can also have an interprocess-compatible associative container and use it with the assoc_container_factory adaptor. I can try to put an example together in the examples section. However, I don't know if this is a common enough need to promote it to lib status.
Maybe this should be managed orthogonally but, can the current design manage with persistent flyweights? Which policies will we reused and which ones added?
In this case, we'd need the factory itself be serializable. The rest of policies wouldn't be affected, AFAICS.
If this is possible and natural, do you think that this could be included in the list of future work?
I can also try to provide an example for this. Whether the scenario has some practical applicability, I don't really know, I'd love to hear from potential users on this one.
Joaquin I understand your concern, if we need to change the core then we recover only some lines of code of the flyweight class. So we can ask ourselfs why not to define a new class?
Any way, let me purpose you an alternative witch do not suffer of "demands a slightly less terse syntax"
// default configuration typedef flyweight<T> fw_t1;
// change core implementation typedef flyweight<T, core<shared_ptr_core> > fw_t2;
// or even with the deduced parameters, change core implementation typedef flyweight<T, shared_ptr_core > fw_t2;
// change some aspect of the default core as now typedef flyweight<T, no_tracking > fw_t3;
This will complicate only the flyweight class which needs to take care of the exclusion of the parameters. And of course this will need a description of what is expected from a core.
This is an interesting approach, transferring the more cumbersome syntax to the core replacement case.Thanks for the suggestion!
I don't know if the Boost.Parameter library takes care of the exclusion aspect?
What do you think?
This could be done with Boost.Parameter, yes. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

----- Original Message ----- Joaquín Mª López Muñoz wrote
"vicente.botet" ha escrito:
For example, can the current design manage with interprocess flyweights? I'm thinking about the possibility to have flyweights fields in a multi_index stored on shared memeory, for example.Which policies will we reused and which ones added? If this is possible and natural, do you think that this could be included in the list of future work?
This can be done by providing an interprocess-compatible holder, factory and locking policy. Tracking would not be affected. Instead of a custom factory, we can also have an interprocess-compatible associative container and use it with the assoc_container_factory adaptor.
I can try to put an example together in the examples section. However, I don't know if this is a common enough need to promote it to lib status.
The example will be enough for me to probe the extension mechanisms.
Maybe this should be managed orthogonally but, can the current design manage with persistent flyweights? Which policies will we reused and which ones added?
In this case, we'd need the factory itself be serializable. The rest of policies wouldn't be affected, AFAICS.
If this is possible and natural, do you think that this could be included in the list of future work?
I can also try to provide an example for this. Whether the scenario has some practical applicability, I don't really know, I'd love to hear from potential users on this one.
I don't know if there are potentials users, but the fact we can do it reinforce the credibility of the current design. So the example will be welcome.
Joaquin I understand your concern, if we need to change the core then we recover only some lines of code of the flyweight class. So we can ask ourselfs why not to define a new class?
Any way, let me purpose you an alternative witch do not suffer of "demands a slightly less terse syntax"
// default configuration typedef flyweight<T> fw_t1;
// change core implementation typedef flyweight<T, core<shared_ptr_core> > fw_t2;
// or even with the deduced parameters, change core implementation typedef flyweight<T, shared_ptr_core > fw_t2;
// change some aspect of the default core as now typedef flyweight<T, no_tracking > fw_t3;
This will complicate only the flyweight class which needs to take care of the exclusion of the parameters. And of course this will need a description of what is expected from a core.
This is an interesting approach, transferring the more cumbersome syntax to the core replacement case.Thanks for the suggestion!
You are welcome
I don't know if the Boost.Parameter library takes care of the exclusion aspect?
What do you think?
This could be done with Boost.Parameter, yes.
I will take a look. Thanks for all --------------------------- Vicente Juan Botet Escriba

Ion Gaztañaga ha escrito:
Hi all,
The formal review of Flyweight library started January 21 and will end January 30, we have a few nice reviews, but I'm sure that those who haven't reviewed it are also interested in the library ;-). The library solves quite a common programming pattern, is small and very well documented. So don't be afraid to try it! Information for reviewers:
I'd like to add that none of the submitted reviews report any actual usage of the lib, except maybe running the tests. It would be great if we can get some feedback about the behavior of the library in some real world scenario, as this kind of info is crucial to determining whether the material has real applicability beyond the toy examples provided. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

Joaquín Mª López Muñoz wrote:
I'd like to add that none of the submitted reviews report any actual usage of the lib, except maybe running the tests. It would be great if we can get some feedback about the behavior of the library in some real world scenario, as this kind of info is crucial to determining whether the material has real applicability beyond the toy examples provided.
I have a situation where I'd like to experiment with this library, but I do not have time to start it yet. Perhaps my mentioning it is half-way valuable, though, so here goes :) When I saw the announcement, I immediately thought about using the library to store and access Unicode text. Specifically, I am imagining that it might allow me to create a string type that acts as a randomly accessible sequence of glyphs. Each glyph would be a flyweight<>. Most glyphs can of course be represented directly by a 32 bit integer, as they're only composed of a single code-point. However, uncommon glyphs that would need more than one code-point (because of combining marks) may benefit from a flyweight approach. Is this a good use case? Would std::basic_string<flyweight<glyph> > "work" (with appropriate character traits)? Could I also get boost's regular expressions to operate on these strings? Kind regards, Edd

----- Mensaje original ----- De: Edd Dawson <lists@mr-edd.co.uk> Fecha: Lunes, Enero 28, 2008 8:29 pm Asunto: Re: [boost] [review] Review of Flyweight library started January 21 and stillrunning! Para: boost@lists.boost.org
I have a situation where I'd like to experiment with this library, but I do not have time to start it yet. Perhaps my mentioning it is half-way valuable, though, so here goes :)
When I saw the announcement, I immediately thought about using the library to store and access Unicode text. Specifically, I am imagining that it might allow me to create a string type that acts as a randomly accessible sequence of glyphs. Each glyph would be a flyweight<>.
Most glyphs can of course be represented directly by a 32 bit integer, as they're only composed of a single code-point. However, uncommon glyphs that would need more than one code-point (because of combining marks) may benefit from a flyweight approach.
Is this a good use case? Would std::basic_string<flyweight<glyph> "work" (with appropriate character traits)? Could I also get boost's regular expressions to operate on these strings?
Hi Edd, I think you'd have problems making this work, but for reasons unrelated to flyweight: for std::basic_string<X> to work, char_traits<X> must have a nested ::int_type typedef with the property that X is convertible to that int_type and back to X without loss of information (http://tinyurl.com/2t3se7 ): If your glyphs contain more than two code-points you'd need an integral type longer than 64 bits, which is most likely unavailable, or else resort to some clever codification of sequences of code-points into 64 bits ints. Other than this, you can certainly use flyweight<> to store glyphs so as to achieve a uniform 32 bits per Unicode character. This can be all wrapped up by a string-like container class. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

This is not a review of the library, rather a wish list. (I started to read it only now.) 1. (Not important) I personally would like a different name: "const_singleton" so. 2. There should be macro BOOST_FLYWEIGHT_NOT_USED which would disable functionality of the library so that flyweight<std::string> would become std::string - for troubleshooting and performance tests. An ability to disable flyweightness per type or tag would be useful. 3. More support for troubleshooting. I would like to see something as macro BOOST_FLYWEIGHT_PARANOIC_MODE where each factory instantiates 2 objects with the same value and the holder returns one or the other, alternating them over and over where possible. This should help to detect wrong use of the flyweight. 4. Support for unit tests run within application session. I would like to see something as: TEST() { ToolThatRevertsHolderOnBlockExitToStateAsNow<std::string> aux; ... use flyweight, e.g. create zillion different values in a stress test } Possibly an ability to temporarily preserve current instances owned by the specified holder, make the holder emply and on exit from the unit test restore the initial state. Ideally, I should have a way to do this with _all_ flyweight holders in the applications, w/o need to specify them explicitly. 5. (This is a biggie) Ability for the end user to specify somewhere the set of allowed shared values created by the factory, by enumeration or with a constraint on those values (only the surnames "Smith", "Jones", "Jackson" and those starting with "Mc") so all other values would be created and used as nonshared. The effects would be: * reduction of the possible unlimited growth of the stored instances, * locking and reference counting overhead for values too hard to share would be eliminated. 6. An API (possibly just in debug mode) which provides usage statistics for a flyweighted type (where available). 7. For people who learn from examples: the more of them the better. The documentation could have a section about potential disadvantages of flyweights. /Pavel

Hello Pavel, thank you for participating in the review! Excuse my late answering, I've been through a couple of hectic days. Pavel Vozenilek ha escrito:
This is not a review of the library, rather a wish list. (I started to read it only now.)
1. (Not important) I personally would like a different name: "const_singleton" so.
Singleton? I don't see how that pattern is related to the flyweight lib.
2. There should be macro BOOST_FLYWEIGHT_NOT_USED which would disable functionality of the library so that flyweight<std::string> would become std::string - for troubleshooting and performance tests.
An ability to disable flyweightness per type or tag would be useful.
Won't it suffice for the user to conditionally typedef her type to flyweight<X> or X depending on some macro of her own? How would a lib-provided facility help more than that?
3. More support for troubleshooting. I would like to see something as macro BOOST_FLYWEIGHT_PARANOIC_MODE where each factory instantiates 2 objects with the same value and the holder returns one or the other, alternating them over and over where possible.
This should help to detect wrong use of the flyweight.
Could you please elaborate. I'm not getting what your inteded usage scenario is. Do you mean duplicating each flyweight value or the factory itself, an to what purpose? [Incidentally, I think you're meaning "factory" instead of "holder", admittedly the terminology can be a bit confusing.]
4. Support for unit tests run within application session. I would like to see something as:
TEST() { ToolThatRevertsHolderOnBlockExitToStateAsNow<std::string> aux;
... use flyweight, e.g. create zillion different values in a stress test }
I don't think that ToolThatReverts... can be sensibly implemented, because of this: For each value in the factory, there are n (with n>=0) flyweight objects pointing to it. I can restore the value, but not the flyweight objects, that live their seprate lives in whatever external context they've been constructed. So, the only thing a factory could do would be to restore values without external references (which can exist only if using the no_tracking policy).
Possibly an ability to temporarily preserve current instances owned by the specified holder, make the holder emply and on exit from the unit test restore the initial state.
Ideally, I should have a way to do this with _all_ flyweight holders in the applications, w/o need to specify them explicitly.
5. (This is a biggie) Ability for the end user to specify somewhere the set of allowed shared values created by the factory, by enumeration or with a constraint on those values (only the surnames "Smith", "Jones", "Jackson" and those starting with "Mc") so all other values would be created and used as nonshared.
The effects would be: * reduction of the possible unlimited growth of the stored instances, * locking and reference counting overhead for values too hard to share would be eliminated.
In principle, a factory adaptor could be done to implement this behavior, with the only caveat that this interferes with the equality semantics issue we've been discussing some days ago. Also, I'm not convinced this would gain any kind of performance, reference counting is almost always cheaper than copying a heavy object like those expected to be flyweighted. Let me put this on my list of things to study.
6. An API (possibly just in debug mode) which provides usage statistics for a flyweighted type (where available).
Yes, this I'll be able to address as part of the future introspection API.
7. For people who learn from examples: the more of them the better.
I've got it in my todo list to keep adding examples. Specific proposals welcome.
The documentation could have a section about potential disadvantages of flyweights.
This will nicely fit in a future performance section that I committed to write.
/Pavel
Best, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo

Hi all, Thank you very much Joaquín , I have learn a lot with your library and the review. Thanks also for all the pertinent answers. Thanks for extending the review period. Ion Gaztañaga wrote:
The formal review of Flyweight library, proposed by Joaquín M. López Muñoz, begins today (sorry for the late announcement): * What is your evaluation of the design?
The library has a simple to use interface based on a understandable domain-specific language having a good configurability and extensibility degree, even if the naming of some policies could be improved. This easy to use interface hides a more complex design. The relations between the different policies are not evident, and even if the library provides a elegant way to extend it, it's no evident to define such extensions without reading the code. Maybe the documentation should include a deeper description of the collaboration of the core implementation and the different policies. I like the way Boost.Parameter is used. In particular the use of is_locking, is_tracking, ... specialization in order to declare extensions. I like the idea to encapsulate in flyweight<T> the acquisition and the release of the shared resources, and as consequence masking the factory initialization. I like also the separation between the specifiers and the implementation classes. * performance test with intermodule_holder The tests performed on example5 should be extended with a intermodule_holder specifier. * intermodule_holder could use static_holder_class As the intermodule_holder is a kind of workaround for platforms where DLL can duplicate static libraries, we can have a better performance on the others for free. On platforms on which static variables are unique inter DLL the intermodule_holder specifier type can be a static_holder_class. This could be controlled using conditional compilation. struct intermodule_holder:holder_marker { template<typename C> struct apply { #ifdef BOOST_NO_DLL_UNIQUE_STATIC_VARIABLE typedef intermodule_holder_class<C> type; #else typedef static_holder_class<C> type; #endif }; }; This variable BOOST_NO_DLL_UNIQUE_STATIC_VARIABLE or a better name should be included in the boost/config.h file. I hope that the boost community could help to find on which platforms BOOST_NO_DLL_UNIQUE_STATIC_VARIABLE must be defined. * holder specifier renaming The holder's main responsibility is to instantiate a unique instance for its internal resources depending on the scope of the flyweight class. So the variation here is the scope. I haven't a better suggestion, but in my opinion holder do not reflects its responsibilities. * concrete holder specifiers renaming I'd RENAME static_holder by module_holder, and intermodule_holder by process_holder. module_holder reflects much more the scope accessibility intent for the users, static_holder talks more about the implementation. So at the end there will be two holders: . module_holder (renaming of static_holder) and . process_holder I don't think that simple_holder is better than static_holder. Does simple_holder stands for simple implemented holder? * factory specifier renaming Maybe repository could be more adapted to the associated responsibilities, store the shared values. * equality semantic It would be great if the library reach to manage with the pathological cases without reducing the performances of the usual cases. If in order to solve the problem you propose the user overrides the operator== with non-& semantics this must be included in the documentation. * adding a exclusive core specifier. In order "to provide maximum customizability so as to meet everybody's needs from basic users to programmers with very specific requirements" I think that the core specifier is needed. // default configuration typedef flyweight<T> fw_t1; // change core implementation typedef flyweight<T, core<shared_ptr_core> > fw_t2; // or even with the deduced parameters, change core implementation typedef flyweight<T, shared_ptr_core > fw_t2; // change some aspect of the default core as now typedef flyweight<T, no_tracking > fw_t3; This complicates only the flyweight class which needs to take care of the exclusion of the parameters. And of course this will need a description of what is expected from a core. As the interface do not change, this could be for future work.
* What is your evaluation of the implementation?
Quite good . Only some details * Minor correction The documentation states that the ..._specifier must be a ..._marker, but this is not needed for the ..._class On the code static_holder_class inherits from holder_marker, and hashed_factory_class inherits from factory_marker. Is this an error without importance, or there is something behind? * refcounted_value is a class that could be public. Why it is declared in the namespace detail. * could you explain why detail::recursive_lightweight_mutex is needed and boost::detail::lightweight_mutex is not used directly? struct simple_locking:locking_marker { typedef detail::recursive_lightweight_mutex mutex_type; typedef mutex_type::scoped_lock lock_type; }; A more general Boost question. Can boost libraries use the details namespace (i.e. private implementation) of the other boost libraries? What is needed to promote this shared private implementations to a public boost library? Should someonepurpose the library?
* What is your evaluation of the documentation?
Good! It was really a pleasure to read the tutorial. Some more elaborated examples should be included in order to show the user how to manage with more complicated examples, for example inter-process flyweights. These examples should be a probe of the extensibility mechanisms and a check the orthogonality of the policies. * intermodule_holder specifier A reference to a document explaining the problem with platforms not ensuring the unicity of static variables when DLL are used will be welcome. * Reference section The reference section was much more hard to follow. I think that the interactions between the different classes are very hard to describe using the current style. Un implementation section will help to understand how all this works, describing the interactions between the core and the policies. * Performance section I expect a performace section to be included. And how the future work will improbe this performances. * Rational Adding of the map versus set problem explanation.
* What is your evaluation of the potential usefulness of the library? Adapted to the described context. The current Flyweight design do not support very well persistent Flyweights, but this is another history.
* Did you try to use the library? With what compiler? No.
* How much effort did you put into your evaluation? In-depth reading of the docs and implementation.
* Are you knowledgeable about the problem domain? Yes, I have already used the Flyweight pattern in the context of mi work applications. Once I used Flyweight on a database context in order to reduce the space used by values that were duplicated too many times.
And finally, every review should answer this question:
* Do you think the library should be accepted as a Boost library? I think that the library proposed by Joaquin should be accepted.
I think that this Flyweight library is useful as it is for some kind off applications. No mayor changes are needed to the interface. Most of the renaming suggestions can be done adding new specifiers and using the extension mechanism. Good luck Joaquín --------------------------- Vicente Juan Botet Escriba

----- Mensaje original ----- De: "vicente.botet" <vicente.botet@wanadoo.fr> Fecha: Domingo, Febrero 3, 2008 12:00 pm Asunto: Re: [boost] [review] Review of Flyweight library started January 21 and still running! Para: boost@lists.boost.org
Hi all,
Thank you very much Joaquín , I have learn a lot with your library and the review. Thanks also for all the pertinent answers.
You're welcome. Thank you for taking the effort to do such a thorough review. [...]
* What is your evaluation of the design?
The library has a simple to use interface based on a understandable domain-specific language having a good configurability and extensibilitydegree, even if the naming of some policies could be improved. This easy to use interface hides a more complex design. The relations between the different policies are not evident, and even if the libraryprovides a elegant way to extend it, it's no evident to define such extensions without reading the code. Maybe the documentation should include a deeper description of the collaboration of the core implementation and the different policies.
Point taken. Will try to improve that part. [...]
* performance test with intermodule_holder The tests performed on example5 should be extended with a intermodule_holder specifier.
I don't see the need: remember the intermodule_holder affects the static initialization process *only*, so the performance of flyweight<T,intermodule_holder> is exactly the same as that of fylweight<T>, save for the startup and shutdown phases of the program.
* intermodule_holder could use static_holder_class As the intermodule_holder is a kind of workaround for platforms where DLL can duplicate static libraries, we can have a better performance on the others for free.
On platforms on which static variables are unique inter DLL the intermodule_holder specifier type can be a static_holder_class. This could be controlled using conditional compilation.
Yes, this is already taken note of. [...]
* holder specifier renaming The holder's main responsibility is to instantiate a unique instance for its internal resources depending on the scope of the flyweight class.
So the variation here is the scope. I haven't a better suggestion, but in my opinion holder do not reflects its responsibilities.
If someone comes up with a better name I'll be happy to change it. I admit the current terminology is not terrific.
* concrete holder specifiers renaming I'd RENAME static_holder by module_holder, and intermodule_holder by process_holder. module_holder reflects much more the scope accessibility intent for the users, static_holder talks more about the implementation.
So at the end there will be two holders: . module_holder (renaming of static_holder) and . process_holder
I don't think that simple_holder is better than static_holder. Does simple_holder stands for simple implemented holder?
* factory specifier renaming Maybe repository could be more adapted to the associated responsibilities,store the shared values.
* equality semantic It would be great if the library reach to manage with the
Yep, this is the idea. As in other terminology issues, I'll do whatever people agree upon. The problem is that in my experience names are something people rarely agree upon :-/ pathological
cases without reducing the performances of the usual cases. If in order to solve the problem you propose the user overrides the operator== with non-& semantics this must be included in the documentation.
This is probably what I'll finally do, given the extremely unlikeliness of the pathological cases.
* adding a exclusive core specifier. [...] As the interface do not change, this could be for future work.
Yep, thanks for your syntax suggestion, this will definitely go to the future work section.
* What is your evaluation of the implementation?
Quite good .
Only some details * Minor correction The documentation states that the ..._specifier must be a ..._marker, but this is not needed for the ..._class On the code static_holder_class inherits from holder_marker, and hashed_factory_class inherits from factory_marker. Is this an error without importance, or there is something behind?
There is something behind :) As you point out, it's only specifiers that need be marked as such. But as a bonus extra, the factory classes provided by the library are also specifiers on their own. How come? You can do so by using MPL lambda expressions, for instance, the specifier: // use std::greater reather than the default std::less set_factory<std::greater<boost::mpl::_2> > can equivalentely be written like this: set_factory_class< boost::mpl::_1,boost::mpl::_2, std::greater<boost::mpl::_2>
You can take a look at the tests, which exercise both forms of specification. I decided not to mention that factory (and holder) classes can act as specifiers because I think readers not very acquainted with MPL might easily get confused (understanding lambda expressions in conenction with the specifiers alone can be already quite a task). This ability used to be documented at the reference, but I've just checked and it is no longer there, I'll restore that bit.
* refcounted_value is a class that could be public. Why it is declared in the namespace detail.
It is an implementation detail, the user won't ever be exposed to it.
* could you explain why detail::recursive_lightweight_mutex is needed and boost::detail::lightweight_mutex is not used directly?
Recursive mutexes are needed when constructing composite complexes with flyweight<>. [...]
A more general Boost question. Can boost libraries use the details namespace (i.e. private implementation) of the other boost libraries?
No, this shouldn't be done. If some detail of lib A is deemed interesting for use in lib B, the proper thing to do is to lift this detail to namespace boost::detail and folder boost/detail. I've done this in the past with some parts of Boost.MultiIndex that are now used by other libraries.
What is needed to promote this shared private implementations to a publicboost library? Should someone purpose the library?
I understand this is what fast track reviews are about. [...]
Some more elaborated examples should be included in order to show the user how to manage with more complicated examples, for example inter-process flyweights. These examples should be a probe of the extensibility mechanisms and a check the orthogonality of the policies.
I'll try to extend the examples section. As it happens, this is one of the tasks I've got most difficulties with, since it's not easy to come up with good example ideas, and I try to keep them fun and motivating if possible.
* intermodule_holder specifier A reference to a document explaining the problem with platforms not ensuring the unicity of static variables when DLL are used will be welcome.
I'm no expert here, but I'd say that platforms without static var unicity across dynamic modules are the majority rather than the exception.
* Reference section The reference section was much more hard to follow. I think that the interactions between the different classes are very hard to describe using the current style.
Well, I know the reference is not the easiest thing to follow, but it tries to keep the level of formality you find at the C++ standard. The idea is you go to the reference for exact, authoritative information. Once you get used to the stiff style, it gets bearable.
Un implementation section will help to understand how all this works, describing the interactions between the core and the policies.
I understand the reader might be curious about the internal implementation, but is this knowledge really needed to use and extend the lib? I'd much prefer to concentrate on improving the part on extending the library without disclosing too much about how things are assembled internally.
* Performance section I expect a performace section to be included. And how the future work will improbe this performances.
Yes, this will be done.
* Rational Adding of the map versus set problem explanation.
I've got something prepared to ask Alberto on this one, please read the following post of mine. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
participants (6)
-
"JOAQUIN LOPEZ MU?Z"
-
Edd Dawson
-
Ion Gaztañaga
-
Joaquín Mª López Muñoz
-
Pavel Vozenilek
-
vicente.botet