headers
Ion Gaztañaga | 21 Jan 21:05
Picon

[review] Review of Flyweight library begins today January 7

The formal review of Flyweight library, proposed by Joaquín M. López 
Muñoz, begins today (sorry for the late announcement):

*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_notes.html
(Continue reading)

Permalink | Reply |
headers
Ion Gaztañaga | 21 Jan 21:23
Picon

Re: [review] Review of Flyweight library begins today January 21

Ion Gaztañaga escribió:
> The formal review of Flyweight library, proposed by Joaquín M. López 
> Muñoz, begins today (sorry for the late announcement):

Obviously, I meant January 21, not January 7 (!!!) so here we go again:

The formal review of Flyweight library, proposed by Joaquín M. López
Muñoz, begins today (sorry for the late announcement):

*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.
(Continue reading)

Permalink | Reply |
headers
Tim Blechmann | 21 Jan 22:31
Favicon

Re: [Boost-announce] [review] Review of Flyweight library begins today January 21

hi all,

> The formal review of Flyweight library, proposed by Joaquín M. López
> Muñoz, begins today (sorry for the late announcement):

<snip>

> * What is your evaluation of the design?

the flyweight class works quite similar to a class, that i am using in
one of my c++ projects. from my point of view, the library is designed
very well, there is only one issue, that i would like to see handled
differently.
the flyweight factory is a shared container, that can be configured to
use one of the locking policies 'no_locking' and 'simple_locking'.
simple_locking would guard the constructor/destructor by a mutually
exclusive lock. while this is certainly correct, it is not actually
necessary. the factory container needs only be locked exclusively by one
thread, when the container is manipulated. 

instead of this 'simple_locking' policy, i would propose to guard the
factory container by a reader-writer lock. if one object is used more
than once, the ctors/dtors only need to acquire a reader lock for the
container, only if new objects are inserted, a writer lock is required.

this policy would make the library more scalable for multi-threaded
applications ... i am not sure about the 'boost way' of implementing
this, but afair a reader-writer lock was about to be added to
boost.thread. this library would be the perfect use case for it ...
(Continue reading)

Permalink | Reply |
headers
Joaquín Mª López Muñoz | 22 Jan 10:10
Picon
Favicon
Gravatar

Re: [Boost-announce] [review] Review of Flyweightlibrarybegins today January 21

Hello Tim, thanks for your review!

Tim Blechmann ha escrito:

[...]

> > * What is your evaluation of the design?
>
> the flyweight class works quite similar to a class, that i am using in
> one of my c++ projects. from my point of view, the library is designed
> very well, there is only one issue, that i would like to see handled
> differently.
> the flyweight factory is a shared container, that can be configured to
> use one of the locking policies 'no_locking' and 'simple_locking'.
> simple_locking would guard the constructor/destructor by a mutually
> exclusive lock. while this is certainly correct, it is not actually
> necessary. the factory container needs only be locked exclusively by one
> thread, when the container is manipulated.
>
> instead of this 'simple_locking' policy, i would propose to guard the
> factory container by a reader-writer lock. if one object is used more
> than once, the ctors/dtors only need to acquire a reader lock for the
> container, only if new objects are inserted, a writer lock is required.
>
> this policy would make the library more scalable for multi-threaded
> applications ... i am not sure about the 'boost way' of implementing
> this, but afair a reader-writer lock was about to be added to
> boost.thread. this library would be the perfect use case for it ...

This idea is very interesting and I will definitely pursue it. A complication
(Continue reading)

Permalink | Reply |
headers
Tim Blechmann | 22 Jan 11:26
Favicon

Re: [Boost-announce] [review] Review of Flyweightlibrarybegins today January 21

> so that we can follow this protocol:
> 
>   readwrite_lock l(mutex);
>   l.read_lock();
>   if(h=find(x))return h;
>   else{
>     l.unlock();
>     l.write_lock();
>     h=insert(x);
>   }

that is similar to my implementation ... 

> An approach to convering both simple and read/write locks would be to
> extend the concepts "Locking Policy" (http://tinyurl.com/ypvy4l ) and
> "Factory" (http://tinyurl.com/2er6dl ) to "ReadWrite Locking Policy" and
> "Lookup Factory", respectively, and only when the components specified
> both conform to the extension concepts would we internally follow the
> readwrite protocol instead of the simple one. I think I can work this
> out, but I'd prefer to put this in the "Future work" section rather than
> trying to implement it immediately, so as to gain some more feedback and
> wait for read/write locks to be brought back into Boost (they were
> removed due to an implementation bug, see http://tinyurl.com/2clcr9 ).

that would be fine with me ... it would definitely make sense to wait for 
the reader-writer locks in boost.thread ...

>> > * Did you try to use the library? With what compiler?
>> >      Did you have any problems?
>>
(Continue reading)

Permalink | Reply |
headers
Alberto Ganesh Barbati | 28 Jan 00:57
Picon
Favicon

Re: [Boost-announce] [review] Review of Flyweightlibrarybegins today January 21

Joaquín Mª López Muñoz ha scritto:
> 
> Tim Blechmann ha escrito:
> 
>> instead of this 'simple_locking' policy, i would propose to guard the
>> factory container by a reader-writer lock. if one object is used more
>> than once, the ctors/dtors only need to acquire a reader lock for the
>> container, only if new objects are inserted, a writer lock is required.
>>
>> this policy would make the library more scalable for multi-threaded
>> applications ... i am not sure about the 'boost way' of implementing
>> this, but afair a reader-writer lock was about to be added to
>> boost.thread. this library would be the perfect use case for it ...
> 
> This idea is very interesting and I will definitely pursue it. A complication
> of using read/write locks is that these do not fit well in the current concept
> framework: a factory f is expected to have the following interface:
> 
>   f.insert(x);
>   f.erase(h);
> 
> where both expressions are *externally* guarded by a regular lock. The
> problem is that f.insert(x) does lookup and optional insertion in one fell
> swoop, and thus does not provide the necessary granularity to use
> a read/write lock. Instead, we'd need something like:
> 
>   f.find(x);
>   f.insert(x);

That would make the interface inefficient, because the search would be
(Continue reading)

Permalink | Reply |
headers
Joaquín Mª López Muñoz | 28 Jan 18:51
Picon
Favicon
Gravatar

Re: [Boost-announce] [review] Review of Flyweight library begins today January 21

Hello Alberto,

Alberto Ganesh Barbati ha escrito:

> Joaquín Mª López Muñoz ha scritto:

[...]

> > This idea is very interesting and I will definitely pursue it. A complication
> > of using read/write locks is that these do not fit well in the current concept
> > framework: a factory f is expected to have the following interface:
> >
> >   f.insert(x);
> >   f.erase(h);
> >
> > where both expressions are *externally* guarded by a regular lock. The
> > problem is that f.insert(x) does lookup and optional insertion in one fell
> > swoop, and thus does not provide the necessary granularity to use
> > a read/write lock. Instead, we'd need something like:
> >
> >   f.find(x);
> >   f.insert(x);
>
> That would make the interface inefficient, because the search would be
> performed twice. I would go with:
>
>   f.find(x);
>   f.insert(h, x);
>
> where h is the value returned by f.find(x). This would exploit the
(Continue reading)

Permalink | Reply |
headers
Alberto Ganesh Barbati | 30 Jan 00:20
Picon
Favicon

Re: [Boost-announce] [review] Review of Flyweight library begins today January 21

Joaquín Mª López Muñoz ha scritto:
> Hello Alberto,
> 
> Alberto Ganesh Barbati ha escrito:
> 
>> That would make the interface inefficient, because the search would be
>> performed twice. I would go with:
>>
>>   f.find(x);
>>   f.insert(h, x);
>>
>> where h is the value returned by f.find(x). This would exploit the
>> two-parameter insert() provided by std::set (and even
>> tr1::unordered_set, for what matters).
> 
> The interface would have to be a little more complicated than that, as
> find() does not return a useful handle (iterator) when the search was
> unsuccesful, it merely returns end(). We'd need to use something like
> lower_bound() for ordered associative containers.
> For unordered associative containers the situation would be even
> worse: although these containers formally accept a hint parameter,
> actually this is useless in containers with unique keys when it is known
> that no equivalent element exists (which is the case here). Furthermore,
> the only memfun usable to get a hint operator would be equal_range
> (no lower_bound in unordered associative containers), and this
> introduces a irregularity with the ordered case.

That is indeed correct. I was too hasty in my proposal.

> All in all, I think using a hint is a lot of hassle for the small potential
(Continue reading)

Permalink | Reply |
headers
Joaquin M Lopez Munoz | 30 Jan 20:54
Picon
Favicon
Gravatar

Re: [Boost-announce] [review] Review of Flyweight library begins today January 21

Alberto Ganesh Barbati <AlbertoBarbati <at> libero.it> writes:

> 
> Joaquín Mª López Muñoz ha scritto:
[...]
> > All in all, I think using a hint is a lot of hassle for the small potential
> > benefit it could bring. However, the only way to know for sure, of course,
> > is measuring it.
> 
> I agree and the benefit could greatly depend on the actual container used.
[...]
> > Why is unlock/lock risky? If I'm getting it right (although I admit to be
> > no expert on threading issues), unlock_upgradable_and_lock() is
> > equivalent to unlocking and then exclusively locking with some additional
> > guarantees, namely that the thread will be the first to obtain exclusive
> > access. The problem is that this can fail (if two threads request the
> > privilege at the same time), hence unlock_upgradable_and_lock()
> > may throw.
> 
> The fact is that if another thread gets write/exclusive access before 
> "our" thread, the hint may become invalid and can no longer be used 
> reliably.

(Assuming we're using a hint, which is debatable as discussed above.)

Umm... I've been rereading N2094 and it proposes the following types
of mutexes:

* Exclusive: plain old mutexes
* Shared: read/write
(Continue reading)

Permalink | Reply |
headers
John Reid | 31 Jan 12:01
Picon

Re: [review] Review of Flyweight library begins today January 21


> * What is your evaluation of the design?
Very good. It seems everything is configurable if needed and works as 
expected out of the box.

Perhaps intermodule_holder should be the default holder specifier as 
there is an unpleasant potential gotcha otherwise? The docs do not state 
why not as far as I can see. An efficiency issue perhaps?

Whilst not having a strong opinion, I prefer the policy-based 
configuration interface as is.

> * What is your evaluation of the implementation?
Whilst not having a strong opinion, I like the operator== as is.

> * What is your evaluation of the documentation?
Extremely clearly written and well thought out. I would like to see more 
explanation of the examples. I like the inclusion of the test code.

> * What is your evaluation of the potential usefulness of the library?
I can think of several situations where I could have gained from using 
the flyweight idiom and this library would have saved me time and 
effort. I expect to encounter more in the future.

> * Did you try to use the library? With what compiler?
No

> * How much effort did you put into your evaluation?
>      A glance? A quick reading? In-depth study?
A quick read through the tutorial and the examples.
(Continue reading)

Permalink | Reply |
headers
Joaquín Mª López Muñoz | 31 Jan 17:56
Picon
Favicon
Gravatar

Re: [review] Review of Flyweight library begins today January 21

Hello John, thank you for submitting your review,

John Reid ha escrito:

> > * What is your evaluation of the design?
> Very good. It seems everything is configurable if needed and works as
> expected out of the box.
>
> Perhaps intermodule_holder should be the default holder specifier as
> there is an unpleasant potential gotcha otherwise? The docs do not state
> why not as far as I can see. An efficiency issue perhaps?

intermodule_holder performs a way more expensive static initialization
process than static_holder (basically, it creates a shared memory
area and interprocess mutex for communication between the different
modules of the program. It only affects static initialization, though,
the rest of operations (flyweight creation and passing around) are exactly
the same as with static_holder.
Another reason for not including this is that Boost.Interprocess (on which
intermodule_holder depends) is not universally supported, so making
intermodule_holder the default would complicate things for some
compilers.

> > * What is your evaluation of the documentation?
> Extremely clearly written and well thought out. I would like to see more
> explanation of the examples. I like the inclusion of the test code.

Any particular example you wished you had more info about? I can
try to improve that if you point me to the improvable parts.
(Continue reading)

Permalink | Reply |
headers
John Reid | 1 Feb 11:38
Picon

Re: [review] Review of Flyweight library begins today January 21

Hello Joaquin,

Joaquín Mª López Muñoz wrote:
> intermodule_holder performs a way more expensive static initialization
> process than static_holder (basically, it creates a shared memory
> area and interprocess mutex for communication between the different
> modules of the program. It only affects static initialization, though,
> the rest of operations (flyweight creation and passing around) are exactly
> the same as with static_holder.
> Another reason for not including this is that Boost.Interprocess (on which
> intermodule_holder depends) is not universally supported, so making
> intermodule_holder the default would complicate things for some
> compilers.
Perhaps this is worth documenting.

>>> * What is your evaluation of the documentation?
>> Extremely clearly written and well thought out. I would like to see more
>> explanation of the examples. I like the inclusion of the test code.
> 
> Any particular example you wished you had more info about? I can
> try to improve that if you point me to the improvable parts.
All of them really, I was hoping to see some of the more important lines 
of code briefly explained in the documentation.

Best,
John.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
(Continue reading)

Permalink | Reply |
headers
Joaquín Mª López Muñoz | 1 Feb 15:51
Picon
Favicon
Gravatar

Re: [review] Review of Flyweight library begins today January21


John Reid ha escrito:

> Hello Joaquin,
>
> Joaquín Mª López Muñoz wrote:
> > intermodule_holder performs a way more expensive static initialization
> > process than static_holder (basically, it creates a shared memory
> > area and interprocess mutex for communication between the different
> > modules of the program. It only affects static initialization, though,
> > the rest of operations (flyweight creation and passing around) are exactly
> > the same as with static_holder.
> > Another reason for not including this is that Boost.Interprocess (on which
> > intermodule_holder depends) is not universally supported, so making
> > intermodule_holder the default would complicate things for some
> > compilers.
> Perhaps this is worth documenting.

I'll add a note on this on the docs.

> >>> * What is your evaluation of the documentation?
> >> Extremely clearly written and well thought out. I would like to see more
> >> explanation of the examples. I like the inclusion of the test code.
> >
> > Any particular example you wished you had more info about? I can
> > try to improve that if you point me to the improvable parts.
> All of them really, I was hoping to see some of the more important lines
> of code briefly explained in the documentation.

OK, I can try to revise this.
(Continue reading)

Permalink | Reply |
headers
Marcus Lindblom | 23 Jan 10:40
Picon

Re: [review] Review of Flyweight library begins today January 7

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?

Very nice. I think the various options are well thougt out and all.

However, I'd like to nit-pick some naming:

* The namespace should be boost::flyweight, without the s, since that's
what the lib & design pattern is called.

* The factory not only creates objects, it also stores them. When I see
'Factory' I think creation only, not storage. How about naming it the
'store' instead, since it seems to be the primary function?

Does the factory even create objects? It doesn't seem like that. It just
stores copies of them in Entrys?

Also, I think that a reader-writer lock would be good (as another poster
wrote), but that's not critical.

> * What is your evaluation of the implementation?

Not looked at thoroughly.

However, there is a dependency on boost::hash<>. Could this be changed
into conforming to a concept that just allows the value to be converted
to size_t?
(Continue reading)

Permalink | Reply |
headers
Joaquín Mª López Muñoz | 23 Jan 14:42
Picon
Favicon
Gravatar

Re: [review] Review of Flyweight library begins today January 7

Hello Marcus, thank you for participating in the review.

Marcus Lindblom ha escrito:

> However, I'd like to nit-pick some naming:
>
> * The namespace should be boost::flyweight, without the s, since that's
> what the lib & design pattern is called.

There is a reason for that spurious 's', namely that some compilers (VC6.0
--though this is probably of little relevance in 2008-- and some versions of
GCC) have problems with using declarations where the namespace and
class names are identical. This issue was raised originally in the context of
Boost.Tuple, and indeed the namespace for that lib is boost::tuples: look up
the section "For those who are really interested in namespaces"at
http://tinyurl.com/ywb9n7 for more info.

> * The factory not only creates objects, it also stores them. When I see
> 'Factory' I think creation only, not storage. How about naming it the
> 'store' instead, since it seems to be the primary function?

The main reason why the name "Factory" was picked is because most
descriptions of the flyweight design pattern use this same terminology:
http://tinyurl.com/ys5k3q . If there's an agreement that the name should be
replaced by something more appropriate, I have no objection to do so, of
course.

> Does the factory even create objects? It doesn't seem like that. It just
> stores copies of them in Entrys?
(Continue reading)

Permalink | Reply |
headers
Marcus Lindblom | 23 Jan 16:08
Picon

Re: [review] Review of Flyweight library begins today January 7

Joaquín Mª López Muñoz wrote:
> Hello Marcus, thank you for participating in the review.
> 
> Marcus Lindblom ha escrito:
> 
>> However, I'd like to nit-pick some naming:
>>
>> * The namespace should be boost::flyweight, without the s, since that's
>> what the lib & design pattern is called.
> 
> There is a reason for that spurious 's', namely that some compilers (VC6.0
> --though this is probably of little relevance in 2008-- and some versions of
> GCC) have problems with using declarations where the namespace and
> class names are identical. This issue was raised originally in the context of
> Boost.Tuple, and indeed the namespace for that lib is boost::tuples: look up
> the section "For those who are really interested in namespaces"at
> http://tinyurl.com/ywb9n7 for more info.

Ok. Fair enough. :)

>> * The factory not only creates objects, it also stores them. When I see
>> 'Factory' I think creation only, not storage. How about naming it the
>> 'store' instead, since it seems to be the primary function?
> 
> The main reason why the name "Factory" was picked is because most
> descriptions of the flyweight design pattern use this same terminology:
> http://tinyurl.com/ys5k3q . If there's an agreement that the name should be
> replaced by something more appropriate, I have no objection to do so, of
> course.
(Continue reading)

Permalink | Reply |
headers
Joaquín Mª López Muñoz | 23 Jan 18:05
Picon
Favicon
Gravatar

Re: [review] Review of Flyweight library begins today January 7

Marcus Lindblom ha escrito:

> Joaquín Mª López Muñoz wrote:
> > Hello Marcus, thank you for participating in the review.
> >
> > Marcus Lindblom ha escrito:

[...]

> >> However, on the reference page, the first header ("boost/flyweight.hpp")
> >> doesn't link to anything?
> >
> > Do you mean the second bullet in the "Contents" section? If so, it links to
> > somewhere below down the same page, maybe that's why it seemed
> > to you to point nowhere. Or are you referring to some other link?
>
> That one, and the first link under "header summary".
>
> I retested, and it works in IE, but not in FireFox.

Oh, I looked again and it's indeed an HTML bug, I had the following destination
anchor

  <a name="#flyweight_synopsis">

instead of

  <a name="flyweight_synopsis">

Seems like IE handled it right, but it's a bug nonetheless. I've corrected it in my
(Continue reading)

Permalink | Reply |
headers
Matias Capeletto | 25 Jan 18:44
Picon
Gravatar

Re: [review] Review of Flyweight library begins today January 7

* What is your evaluation of the design?

Great. I specially like the simplicity of the out of the box version,
that will surely cover most of the uses cases.

I like the current design of configuration parameters.
With respect to Equality semantics, I have no strong opinion about
this particular item. I think that the gain in using the &-version is
too good to be discarded, but I do not like the fact of that small
possibility of a user being surprised by it. A warning box can be put
in the docs or this gain can be activated by the user overloading the
operator== in a per class basis as Joaquin proposed in other thread (A
section will be needed in the docs about this and maybe a macro to
make it easy to activate it).
In the future work section Joaquin talks about an introspection API, I
think this API could be very useful for debugging purpose.

* What is your evaluation of the implementation?

I gave it a quick look. The code is very clean and is already bundle
with the machinery to pass over compiler quirks. I think that is great
how this library rest over the shoulders of other libraries of boost.
It has a very nice test suit too.

* What is your evaluation of the documentation?

Good. It will be cool to have more info about when to use a set
instead of a hashed container for the factory.

{{
(Continue reading)

Permalink | Reply |
headers
JOAQUIN LOPEZ MU?Z | 27 Jan 20:23
Picon
Favicon
Gravatar

Re: [review] Review of Flyweight library begins today January 7

Hello Matías, excuse my late answering, I've been
Internet deprived for the best part of the weekend.

----- Mensaje original -----
De: Matias Capeletto <matias.capeletto <at> gmail.com>
Fecha: Viernes, Enero 25, 2008 6:45 pm
Asunto: Re: [boost] [review] Review of Flyweight library begins	today 
January 7
Para: boost <at> lists.boost.org

> * What is your evaluation of the design?
> 
> Great. I specially like the simplicity of the out of the box version,
> that will surely cover most of the uses cases.
> 
> I like the current design of configuration parameters.
> With respect to Equality semantics, I have no strong opinion about
> this particular item. I think that the gain in using the &-version is
> too good to be discarded, but I do not like the fact of that small
> possibility of a user being surprised by it. A warning box can be put
> in the docs or this gain can be activated by the user overloading the
> operator== in a per class basis as Joaquin proposed in other 
> thread (A section will be needed in the docs about this and maybe a
> macro to make it easy to activate it).

Another option would be to make the &-version the default and
inform the user about the possibility to override operator== so
as to implement the non-&-version... I'm very undecided about this.

[...]
(Continue reading)

Permalink | Reply |

Gmane