[flyweight] post-review version available
Hello, I'm happy to announce the availability of the post-review version of Boost.Flyweight: *Online docs:* http://tinyurl.com/2sstyr http://svn.boost.org/svn/boost/sandbox/flyweight/libs/flyweight/index.html *Download as .zip:* http://tinyurl.com/hrdm6 http://www.boost-consulting.com/vault/index.php?&direction=0&order=&directory=Patterns *Check out from SVN:* https://svn.boost.org/svn/boost/sandbox/flyweight *Tested with:* MSVC++ 8.0 and GCC 3.4.4. Compilation reports most welcome. Older compilers like MSVC++ 6.5 are no longer supported. *What's new:* I've tried to address most of the suggestions and comments raised during the review. The two most important changes are: 1. Alberto Barbati's insightful review has led to the introduction of so-called key-value flyweights (http://tinyurl.com/5mnwjp ). This type of flyweights is meant for the situations where constructing the underlying type is expensive and must be avoided except if absolutely necessary. I think the design is robust enough and covers the variants (inner/outer key) that arose during the discussion with Alberto. 2. A comprehensive performance section is provided (http://tinyurl.com/5ap5kt ). *What's pending* 1. Some of the suggestions given at the review, like reducing intermodule_holder to static_holder when permissible (Vicente Botet) or studying read/write locking policies are still pending or have been postponed to later releases of the library. I felt the current state is sufficiently robust for publication. 2. Serialization support is currently broken, pending the resolution of a proposed feature to add to Boost.Serialization (run-time archive helpers). I'd be very grateful if you can take a look at the post- review version of Boost.Flyweight (specially if you were a reviewer), test it and comment on it, with emphasis on the new features like key-value flyweights. If people are happy with this revision my target release for inclusion will be Boost 1.38. Thank you, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
________________________________________ De: boost-bounces@lists.boost.org [boost-bounces@lists.boost.org] En nombre de JOAQUIN M. LOPEZ MUÑOZ [joaquin@tid.es] Enviado el: lunes, 01 de septiembre de 2008 18:26 Para: boost@lists.boost.org Asunto: [boost] [flyweight] post-review version available
Hello,
I'm happy to announce the availability of the post-review version of Boost.Flyweight:
[...]
*Tested with:*
I forgot to mention that this version has to be built against the trunk version of Boost (it uses boost::swap, which hasn't made it to any Boost release yet). Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
JOAQUIN M. LOPEZ MUÑOZ wrote:
*What's new:*
I've tried to address most of the suggestions and comments raised during the review. The two most important changes are: 1. Alberto Barbati's insightful review has led to the introduction of so-called key-value flyweights
I always wondered why Flyweight didn't do it, since this seems like the best possible use to me, since it really acts like a named resource manager. That's really great news, and should make Flyweight fairly popular in the game development world.
JOAQUIN M. LOPEZ MUÑOZ skrev:
I've tried to address most of the suggestions and comments raised during the review. The two most important changes are: 1. Alberto Barbati's insightful review has led to the introduction of so-called key-value flyweights (http://tinyurl.com/5mnwjp ). This type of flyweights is meant for the situations where constructing the underlying type is expensive and must be avoided except if absolutely necessary. I think the design is robust enough and covers the variants (inner/outer key) that arose during the discussion with Alberto.
I just read the start of the tutorial, and I like what I see. I haven't tried using the library yet, though. But I have a question regarding the last assignment in the "Key extractors" section. Maybe I just missed something, so please enlighten me. It's nice that the following construct will avoid creating a texture if it already exists: flyweight<key_value<std::string,texture> > fw("grass.texture"); But in the example with the texture_filename_extractor, it seems to me, that a texture is still fully constructed before the assignment. From the documentation (http://svn.boost.org/svn/boost/sandbox/flyweight/libs/flyweight/doc/tutorial...): struct texture_filename_extractor {...}; flyweight<key_value<std::string,texture, texture_filename_extractor> > fw; ... fw=texture("sand.texture"); // OK now I get it that this is to show the need for key extractors, but isn't it possible to also assign without creating already existing textures? I got a little confused, because the previous section dealt with improving performance by avoiding unnecessary construction, and here it occurs to me that there is one. Wouldn't the optimal way to perform this assignment be something like: typedef flyweight<key_value<std::string,texture, texture_filename_extractor> > texture_fw; texture_fw fw; ... fw=texture_fw("sand.texture"); // No unnecessary construction? If this is the case, I think it would improve the documentation if a note was added after the example that this is the best way to do an assignment from a "new" texture. Please correct me if I'm wrong. Best regards, Christian
Hi Christian, ________________________________________ De: boost-bounces@lists.boost.org [boost-bounces@lists.boost.org] En nombre de Christian Larsen [contact@dword.dk] Enviado el: martes, 02 de septiembre de 2008 21:55 Para: boost@lists.boost.org Asunto: Re: [boost] [flyweight] post-review version available
JOAQUIN M. LOPEZ MUÑOZ skrev:
I've tried to address most of the suggestions and comments raised during the review. The two most important changes are: 1. Alberto Barbati's insightful review has led to the introduction of so-called key-value flyweights (http://tinyurl.com/5mnwjp ). [...]
I just read the start of the tutorial, and I like what I see. I haven't tried using the library yet, though. But I have a question regarding the last assignment in the "Key extractors" section.[...]
struct texture_filename_extractor {...}; flyweight<key_value<std::string,texture, texture_filename_extractor> > fw; ... fw=texture("sand.texture"); // OK now
I get it that this is to show the need for key extractors, but isn't it possible to also assign without creating already existing textures?[...] Wouldn't the optimal way to perform this assignment be something like:
typedef flyweight<key_value<std::string,texture, texture_filename_extractor> > texture_fw; texture_fw fw; ... fw=texture_fw("sand.texture"); // No unnecessary construction?
Yep, you're absolutely correct. The suboptimal assignment 'fw= texture("sand.texture")' is used just to show where key extractors are needed.
If this is the case, I think it would improve the documentation if a note was added after the example that this is the best way to do an assignment from a "new" texture. Please correct me if I'm wrong.
Yes, the docs can be misleading here. I'll either clarify the point in the docs or else rewrite the snippet in a way that makes it impossible to optimize the expression, something like void foo(const texture& x) { flyweight<key_value<std::string,texture> > fw(x); } Thanks for your feedback, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
JOAQUIN M. LOPEZ MUÑOZ skrev:
I've tried to address most of the suggestions and comments raised during the review. The two most important changes are: 1. Alberto Barbati's insightful review has led to the introduction of so-called key-value flyweights (http://tinyurl.com/5mnwjp ). This type of flyweights is meant for the situations where constructing the underlying type is expensive and must be avoided except if absolutely necessary. I think the design is robust enough and covers the variants (inner/outer key) that arose during the discussion with Alberto.
I had a preliminary look at the key/value stuff of the revised flyweight library. I assume that the rest has not undergone severe modifications so I concentrated myself on that part. First of all, let me congratulate with the good job. It really is a big improvement in addressing a scenario I faced (too) many times in the past. Here's my first thoughts about the new feature: 1) I like the idea of having a flyweight<key_value<K,V>> rather than flyweight<K,V>. It is more consistent with the rest of the framework 2) It's good to have an optional key extractor 3) You assume that Value is constructible from Key. This initially stroke me as a big restriction, but in fact in case you have a class that doesn't have a suitable constructor, you can always wrap it. For example: class ThirdPartyTextureClass // I can't modify this class { public: ThirdPartyTextureClass(const char* filename, /* other params here */); /* ... */ }; // can't have flyweight<key_value<std::string, ThirdPartyTextureClass> > struct texture_wrap : ThirdPartyTextureClass { texture_wrap(const std::string& filename) : ThirdPartyTextureClass(filename.c_str(), /* ... */) {} }; // now this is good: flyweight<key_value<std::string, texture_wrap> > f; I believe this might be mentioned in the docs, if you have time for that. Another approach that would make sense is to add a "factory functor" argument in addition to the "key extractor". This would help in cases where derivation is impossible or would look odd, for example if the value type is a basic type or a smart pointer. 4) I don't understand why you need Value to be Assignable. I believe it's really important that you could support non-Copiable and non-Assignable value types. The main reason for using flyweight is precisely to avoid having more copies of the value than the strictly necessary. If you are going to copy values, you are failing this requirement. 5) (probably related with point 4) I don't understand the role of copy_value() HTH, Ganesh
Alberto Ganesh Barbati escribió:
JOAQUIN M. LOPEZ MUÑOZ skrev:
I've tried to address most of the suggestions and comments raised during the review. The two most important changes are: 1. Alberto Barbati's insightful review has led to the introduction of so-called key-value flyweights (http://tinyurl.com/5mnwjp ). This type of flyweights is meant for the situations where constructing the underlying type is expensive and must be avoided except if absolutely necessary. I think the design is robust enough and covers the variants (inner/outer key) that arose during the discussion with Alberto.
I had a preliminary look at the key/value stuff of the revised flyweight library. I assume that the rest has not undergone severe modifications so I concentrated myself on that part.
The other major addition is the section on performance: http://svn.boost.org/svn/boost/sandbox/flyweight/libs/flyweight/doc/performa...
First of all, let me congratulate with the good job. It really is a big improvement in addressing a scenario I faced (too) many times in the past. Here's my first thoughts about the new feature:
1) I like the idea of having a flyweight<key_value<K,V>> rather than flyweight<K,V>. It is more consistent with the rest of the framework
It's also less work for the lib maintainer and less conceptual load for the user, I hope, than having two separate class templates.
2) It's good to have an optional key extractor
This is meant to address the inner/outer key dilemma, so that you can have both approaches.
3) You assume that Value is constructible from Key. This initially stroke me as a big restriction, but in fact in case you have a class that doesn't have a suitable constructor, you can always wrap it. For example:
class ThirdPartyTextureClass // I can't modify this class { public: ThirdPartyTextureClass(const char* filename, /* other params here */); /* ... */ };
// can't have flyweight<key_value<std::string, ThirdPartyTextureClass> >
struct texture_wrap : ThirdPartyTextureClass { texture_wrap(const std::string& filename) : ThirdPartyTextureClass(filename.c_str(), /* ... */) {} };
// now this is good: flyweight<key_value<std::string, texture_wrap> > f;
I believe this might be mentioned in the docs, if you have time for that. Another approach that would make sense is to add a "factory functor" argument in addition to the "key extractor". This would help in cases where derivation is impossible or would look odd, for example if the value type is a basic type or a smart pointer.
Allowing for key->value customization is something I definitely have to put in my future work section. I preferred not to include anything yet so as to not overdesign things and gather user feedback. Such a "factory functor" would be a rather special beast rather than a regular factory, since it should have to do in-place construction of Value, much as boost::in_place_factory does (http://tinyurl.com/4dqzoc ).
4) I don't understand why you need Value to be Assignable. I believe it's really important that you could support non-Copiable and non-Assignable value types. The main reason for using flyweight is precisely to avoid having more copies of the value than the strictly necessary. If you are going to copy values, you are failing this requirement.
For key-value flyweights, Value is *not* required to be assignable or copy constructible, *except* if assignment or construction from value is attempted: flyweight<key_value<key_type,value_type> > fw; value_type v; fw=v; // for this to work value_type must be assignable. (See also http://tinyurl.com/create.php ) So, if you stay away from these expressions the only requisite on Value is that it must be constructible from Key. I expect that these expressions will be rare, precisely because we're having a key-flyweight value to avoid value copying (among other things), but this is up to the user.
5) (probably related with point 4) I don't understand the role of copy_value()
This internal function is only used at the situations described above. Hope this clarifies the point on assignability of Value. Thank you very much for re-reviewing the lib! Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
joaquin@tid.es ha scritto:
Alberto Ganesh Barbati escribió:
4) I don't understand why you need Value to be Assignable. I believe it's really important that you could support non-Copiable and non-Assignable value types. The main reason for using flyweight is precisely to avoid having more copies of the value than the strictly necessary. If you are going to copy values, you are failing this requirement.
For key-value flyweights, Value is *not* required to be assignable or copy constructible, *except* if assignment or construction from value is attempted:
flyweight<key_value<key_type,value_type> > fw; value_type v; fw=v; // for this to work value_type must be assignable.
(See also http://tinyurl.com/create.php )
I see. That is very good. However you should correct the documentation, because it now says clearly in key_value.html: "Let Key be a type with some implicit equivalence relationship and Value an Assignable type constructible from Key." HTH, Ganesh
Alberto Ganesh Barbati escribió:
joaquin@tid.es ha scritto:
For key-value flyweights, Value is *not* required to be assignable or copy constructible, *except* if assignment or construction from value is attempted:
flyweight<key_value<key_type,value_type> > fw; value_type v; fw=v; // for this to work value_type must be assignable.
(See also http://tinyurl.com/create.php )
I see. That is very good. However you should correct the documentation, because it now says clearly in key_value.html: "Let Key be a type with some implicit equivalence relationship and Value an Assignable type constructible from Key."
Ouch! Yes, this is incorrect, thanks for spotting it, will fix it. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
I tried to compile the example perf.cpp but I needed to link to the pthread library under linux. Otherwise I will get errors like: perf.cpp:(.text._ZN5boost10flyweights6detail27recursive_lightweight_mutexC1Ev[boost::flyweights::detail::recursive_lightweight_mutex::recursive_lightweight_mutex()]+0x1e): undefined reference to `pthread_mutexattr_init' Additionally the loop that tests value access time was optimized away so I had to print the value 's' to cout as a quick fix to this. Here are a few results on an Athlon XP 2000+, g++-4.3.2. I created a file that contained the don quichote text four times and used that as input. simple string ------------------------- initialization time: 5.33 s assignment time: 13.81 s equality comparison time: 0.42 s value access time: 14.8 s bytes used: 294747496 flyweight, hashed factory------------ initialization time: 6.39 s assignment time: 5.81 s equality comparison time: 0.42 s value access time: 11.54 s bytes used: 241724917= flyweights(240691328)+values(1033589) flyweight, hashed factory, no tracking------------ initialization time: 5.98 s assignment time: 1.32 s equality comparison time: 0.43 s value access time: 11.54 s bytes used: 241622381= flyweights(240691328)+values(931053) flyweight, set-based factory-------------------- initialization time: 7.43 s assignment time: 6.05 s equality comparison time: 0.43 s value access time: 13.34 s bytes used: 241836109= flyweights(240691328)+values(1144781) flyweight, set-based factory, no tracking--------- initialization time: 7.22 s assignment time: 1.34 s equality comparison time: 0.43 s value access time: 12.17 s bytes used: 241733573= flyweights(240691328)+values(1042245)
Hello Kevin, ________________________________________ De: boost-bounces@lists.boost.org [boost-bounces@lists.boost.org] En nombre de Kevin Sopp [baraclese@googlemail.com] Enviado el: viernes, 07 de noviembre de 2008 23:34 Para: boost@lists.boost.org Asunto: Re: [boost] [flyweight] post-review version available
I tried to compile the example perf.cpp but I needed to link to the pthread library under linux. Otherwise I will get errors like:
perf.cpp:(.text._ZN5boost10flyweights6detail27recursive_lightweight_mutexC1Ev [boost::flyweights::detail::recursive_lightweight_mutex::recursive_lightweight_mutex()]+0x1e): undefined reference to `pthread_mutexattr_init'
Additionally the loop that tests value access time was optimized away so I had to print the value 's' to cout as a quick fix to this.
Thank you for spotting these two quirks. Would you mind repeating your tests using the replacement files attached? boost/flyweight/detail/recursive_lw_mutex.hpp libs/flyweight/example/perf.cpp Hopefully the problems will go away now. Please report back. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
----- Original Message ----- From: "JOAQUIN M. LOPEZ MUÑOZ" <joaquin@tid.es> To: <boost@lists.boost.org> Sent: Monday, September 01, 2008 6:26 PM Subject: [boost] [flyweight] post-review version available
Hello,
I'm happy to announce the availability of the post-review version of Boost.Flyweight:
*What's new:*
I've tried to address most of the suggestions and comments raised during the review. The two most important changes are: 1. Alberto Barbati's insightful review has led to the introduction of so-called key-value flyweights (http://tinyurl.com/5mnwjp ). This type of flyweights is meant for the situations where constructing the underlying type is expensive and must be avoided except if absolutely necessary. I think the design is robust enough and covers the variants (inner/outer key) that arose during the discussion with Alberto. 2. A comprehensive performance section is provided (http://tinyurl.com/5ap5kt ).
*What's pending*
1. Some of the suggestions given at the review, like reducing intermodule_holder to static_holder when permissible (Vicente Botet) or studying read/write locking policies are still pending or have been postponed to later releases of the library. I felt the current state is sufficiently robust for publication. 2. Serialization support is currently broken, pending the resolution of a proposed feature to add to Boost.Serialization (run-time archive helpers).
I'd be very grateful if you can take a look at the post- review version of Boost.Flyweight (specially if you were a reviewer), test it and comment on it, with emphasis on the new features like key-value flyweights. If people are happy with this revision my target release for inclusion will be Boost 1.38.
Hi Joaquin, I have take a look to the library. The way you have integrated the Key-value in the flywight class is very elegant. I have just some questions related to the integration of flywight<T> in multi_index. Can we use flywight<T> as a member field of a multi_index container? what about when the multi_index container is in shared memory? Thanks for the clear performance section you have added. You say "Actually the types tested are not exactly those listed above, but instrumented versions that keep track of the allocated memory for profiling purposes. " These instrumented versions can be also useful to the user to make its own messures, are them available somewhere? Do you plan to manage the following items on the release 1.38 * On platforms on which static variables are unique inter DLL the intermodule_holder specifier type can be a static_holder_class. * Add an exclusive core specifier (see my review) Otherwise, could you add them to the Future Work section? I've runned the test and the examples on - g++ (GCC) 3.4.4 (cygming special, gdc 0.12, using dmd 0.125) Every think works well except the serialization test and example are broken. How do you plan to manage this issue as Robert do not plan to take in account the helper you need? Kind regards, ______________________ Vicente Juan Botet Escribá
Hi Vicente, ________________________________________ De: boost-bounces@lists.boost.org [boost-bounces@lists.boost.org] En nombre de vicente.botet [vicente.botet@wanadoo.fr] Enviado el: martes, 09 de septiembre de 2008 7:48 Para: boost@lists.boost.org Asunto: Re: [boost] [flyweight] post-review version available
----- Original Message ----- From: "JOAQUIN M. LOPEZ MUÑOZ" <joaquin@tid.es> To: <boost@lists.boost.org> Sent: Monday, September 01, 2008 6:26 PM Subject: [boost] [flyweight] post-review version available
Hello,
I'm happy to announce the availability of the post-review version of Boost.Flyweight: [...]
Hi Joaquin,
I have take a look to the library.
The way you have integrated the Key-value in the flywight class is very elegant.
Thank you!
I have just some questions related to the integration of flywight<T> in multi_index. Can we use flywight<T> as a member field of a multi_index container?
Certainly. For instance: typedef flyweight<std::string> fw_string; struct element { fw_string x; fw_string y; element(const char* x,const char* y):x(x),y(y){} }; typedef multi_index_container< element, indexed_by< ordered_unique< member<element,fw_string,&element::x> >, ordered_non_unique< member<element,fw_string,&element::y> > >
multi_t;
int main() { multi_t m; m.insert(element("hello","boost")); m.insert(element("bye","world")); assert(m.find(fw_string("hello"))->y=="boost"); }
what about when the multi_index container is in shared memory?
For this to work you'd have to have flyweight<> itself placeable in shared memory, which currently is not the case :( This would be a tough exercise, as shared-memory compatibility affects all the components of flyweight: factory, tracking, holder, locking.
Thanks for the clear performance section you have added. You say "Actually the types tested are not exactly those listed above, but instrumented versions that keep track of the allocated memory for profiling purposes. "
These instrumented versions can be also useful to the user to make its own messures, are them available somewhere?
Yes, at the performance program source code, see http://tinyurl.com/66df5k .
Do you plan to manage the following items on the release 1.38 * On platforms on which static variables are unique inter DLL the intermodule_holder specifier type can be a static_holder_class.
Yes, this is something I still have in my todo list and plan to address in the following weeks. The problem is that I don't know where to ask about such platforms: I usually work with Windows where this situation does not happen; Are you acquainted with some environment where static vars don't get duplicated across DLLs? If so please contact me offlist and we can work something out.
* Add an exclusive core specifier (see my review) Otherwise, could you add them to the Future Work section?
This is not currently in my todo list. I'd prefer to wait and see how the current Boost.Flyweight version fares in the real world before deciding on the convenience of adding the core stuff. Having such capability from the start could be overdesigning things and exposes a lot of internal detail, which is something I always try to do with care.
I've runned the test and the examples on - g++ (GCC) 3.4.4 (cygming special, gdc 0.12, using dmd 0.125)
Every think works well except the serialization test and example are broken. How do you plan to manage this issue as Robert do not plan to take in account the helper you need?
Well, I'm afraid B.F most likely will ship in 1.38 without serialization support. My plan is to begin working on adding helper support to Boost.Serialization and eventually gather evidence of its usefulness as a core Archive concept. When/if this happens I'll add serialization capabilities to B.F, but I doubt this'll develop before 1.38. Thank you very much for your post-review review. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
participants (7)
-
Alberto Ganesh Barbati -
Christian Larsen -
JOAQUIN M. LOPEZ MUÑOZ -
joaquin＠tid.es
-
Kevin Sopp -
Mathias Gaunard -
vicente.botet