[boost] rw_mutex changes

From: Michael Glassford (glassfordm_at_[hidden])
Date: 2003-10-22 10:49:20

Quite some time ago I made some modifications to the rw_mutex and rw_lock
classes that can now be found in the "thread_dev" branch of Boost.Thread (at
the time I made the changes, these classes were not in CVS). I corresponded
privately with William Kempf (author of Boost.Thread but not of these
classes) about the changes, and he said he would be interested in looking at
them; however, several high-priority projects came up at work and I was
never able to finish them. I was able to finish the changes a few months
ago, but he hasn't responded to any of my messages (I've noticed he hasn't
been posting recently either), so I thought I would ask here: is there any
interest in these changes? And, if they turn out to be worthwhile enough,
what would be the appropriate way of getting them into CVS?

Below is a list of what I did and what remains to be done. I'll be glad to
make the code itself available in whatever way is most appropriate.

Mike

New features:
-------------

Finish the partially-implemented lock demotion and promotion features via
the demote(), try_demote(), timed_demote(), try_promote(), and
timed_promote() member functions on the appropriate lock classes. There is
intentionally no promote() member function.

I should note that William Kempf expressed reservations about lock promotion
and demotion. His precise comment was:

> That's a hot topic.  I'm not sure the feature is really that useful since
> it will provide only a minor performance boost in comparison to simply
> unlocking and relocking with the new type.  I'm hesitant to even look into
> this, since nearly every implementation that's been posted to the various
> groups has been proven to be incorrect, illustrating that this is a
> difficult thing to implement correctly (and once you do, is there any real
> performance increase?).

To which my reply was:

> The primary advantage I see in lock demotion is that it allows a thread
> to acquire a write lock to a resource, modify the resource, then demote
> itself to a read lock, thereby allowing other readers to stop waiting
> but preventing other writers from making modifications; by doing this,
> the thread knows that the modifications it just made are still valid and
> can use them without additional checking, which would not be the case if
> it released the write lock and then acquired a read lock. If the
> operation performed after the demotion is fairly lengthy, the releasing
> of other readers could result in improved throughput.

Whether the implementation is correct or not is something I would appreciate
feedback on, as I'm not aware of what the other implementations or there
problems were.

Addition of set_lock(), try_set_lock(), and timed_set_lock() members; you
simply tell it what lock type you want (read locked, write locked, or
unlocked) and it "does the right thing" (using whatever combination of
promotion, demotion, unlocking, and locking is necessary) to get that lock
type from the current lock type.

Other changes:
--------------

Renamed SHARED_LOCK and EXCL_LOCK constants to READ_LOCK and WRITE_LOCK. It
seemed inconsistent to have class methods (e.g. rdlock(), wrlock())
referring to read and write locks and to have constants referring to shared
and exclusive locks.

Changed scoped_try_rw_lock constructor so that the parameters indicate both
what kind of initial lock is desired and whether it should block until it
can obtain that lock. This seems more in line with the "try" nature of the
class.

Changed scoped_timed_rw_lock constructor so that the parameters indicate
both what kind of initial lock is desired and how long it should block to
obtain that lock if necessary. This seems more in line with the "timed"
nature of the class.

I'm pretty sure that, at the time I made all these changes, the
documentation stated that the ScopedTimedRWLock concept was an extension of
the ScopedTryRWLock concept, but the scoped_timed_rwlock class didn't
implement the scoped_try_lock methods (try_rd_lock(), try_wr_lock(), etc.).
I fixed this issue by added the missing methods; the documentation fixed it
by saying that the ScopedTimedRWLock is an extension of the RWLock concept
instead of the ScopedTryRWLock concept. I like the original concept
definitions better (a "try" operation is very similar to a "timed" operation
with a wait time of zero), but there may be other reasons that I'm not aware
of to prefer the current definition.

Modified the scheduling algorithm to fix issues with the new
promotion/demotion feature.

Renamed a couple of private/protected member variables to better indicate
their function.

To do:
------

Update documentation to match changes

Add unit tests for the new features

Rename rdlock() member function to read_lock() and wrlock() to write_lock()?

Add locked() and state() member functions to the rw_mutex class? Probably
not: although it is possible to implement these functions given the current
mutex & lock implementation, there other implementations that may not be
able to provide this information.

Various cleanup tasks.