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

From: Alberto Ganesh Barbati (AlbertoBarbati_at_[hidden])
Date: 2008-01-27 18:57: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
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).

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

which would become (using promote() instead of the very risky unlock/lock):

    readwrite_lock l(mutex);
    l.read_lock();
    if(h=find(x))return h;
    else{
      l.promote();
      h=insert(h, 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 ).
> 

I believe you should consider using the find/insert approach immediately
and not wait for readwrite locks. If you do so, you may upgrade to
readwrite locks later once they become available, while sticking to the
insert-only approach will make the upgrade more difficult because a 
change in the concept framework might break existing code (for example 
think about uses of assoc_container_factory with a user-provided container).

One way to implement this could be:

    typename LockingPolicy::lock_type l(mutex);
    if(h=find(x))return h;
    else{
      LockingPolicy::promote(l);
      h=insert(h, x);
    }

requiring the locking policy functions to provide *at least* read
lock after the lock construction and to ensure write lock after a call 
to promote(). For regular mutexes promote() can simply be a no-op, of
course. If the users had their own non-boost (and hopefully not buggy)
read write mutex classes they could plug them in easily.

Just my two eurocent,

Ganesh