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

From: Joaquín Mª López Muñoz (joaquin_at_[hidden])
Date: 2008-01-22 04:10:58

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
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);

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);
  }

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 ).

[...]

> > * Did you try to use the library? With what compiler?
> >      Did you have any problems?
>
> no

Given that you  are already using a lib of your own around the same
concept, I think it would be very interesting if you could try replacing
that with my lib and report whether the experience is smooth enough
--in particular, if you're using read/write locks you can measure whether
thery make an actual diference wrt to candidate Boost.Flyweight simple
locks.

[...]

Best,

Joaquín M López Muñoz
Telefónica, Investigación y Desarrollo