[flyweight] Review
* What is your evaluation of the design? I like ability for the drop in replacement of const T (with assignability). It makes it easy to test whether it makes a big difference in run time scenarios, before committing to its use. I liked the policy based interface. * What is your evaluation of the implementation? I had a small hunt around the code and liked the modularity of it. A friend of mine railed against the default use of a static factory. He would prefer, by default, to control the lifetime of the factory, but that would make it harder for "drop in" usage, and there is the flexibility of overriding the holder class. * What is your evaluation of the documentation? I didn't read it all closely, but I certainly understood what the library was for and how to use it after viewing the docs. Of course, real world mileage will vary, but some performance indicators or at least discussion of the tradeoff of extra hashing vs reduced dynamic allocations would be useful, as well as any other performance impacts (how big should the data set be before using it, what percentage of unique values makes it worthwhile). * What is your evaluation of the potential usefulness of the library? Useful to have available. Unless there is a no brainer scenario, it seems that real world testing on live data will be needed to evaluate it's value in any given scenario? * Did you try to use the library? With what compiler? Did you have any problems? I didn't try the library, since I am not using 1.35. But I am keen to use it when 1.35 is released. * How much effort did you put into your evaluation? A moderate investigation. * Are you knowledgeable about the problem domain? No experience making an implementation, but knowledgable about the problem. * Do you think the library should be accepted as a Boost library? Yes Neil Hunt
Hello Neil, thanks for your review!
Neil Hunt
* What is your evaluation of the design? I like ability for the drop in replacement of const T (with assignability). It makes it easy to test whether it makes a big difference in run time scenarios, before committing to its use.
Well, have in mind drop in replacement only works in environments where implicit convertibility to T is allowed. For instance, the following won't compile: flyweightstd::string s; ... s.append(...); You'll have to write instead: flyweightstd::string s; ... s.get().append(...); If only we had smart references in C++ :( [...]
Of course, real world mileage will vary, but some performance indicators or at least discussion of the tradeoff of extra hashing vs reduced dynamic allocations would be useful, as well as any other performance impacts (how big should the data set be before using it, what percentage of unique values makes it worthwhile).
I agree with you performance is likely very dependent on the particular usage scnario, but anyway the section you propose makes a lot of sense, so I'll put it in my todo list. FWIW, there is a performance measuring example that you can play with at: http://svn.boost.org/svn/boost/sandbox/flyweight/libs/flyweight/doc/examples... ml#example5
* What is your evaluation of the potential usefulness of the library? Useful to have available. Unless there is a no brainer scenario, it seems that real world testing on live data will be needed to evaluate it's value in any given scenario?
I couldn't agree more. I was hoping some of the reviewers had some real scenario of her own where to put the lib in practice. Best regards, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
participants (2)
-
Joaquín M López Muñoz
-
Neil Hunt