Re: Pool Review.

From: William Kempf (sirwillard_at_[hidden])
Date: 2000-11-30 10:34:55

--- In boost_at_[hidden], "Peter Dimov" <pdimov_at_m...> wrote:
> My pool comments:
> 
> I think that pool should be accepted into boost.
> 
> Code:
> 
> * headers end in .h, not .hpp;
> 
> * several changes were necessary to get pool to compile with MSVC 
6sp4
> (default STL) and g++ 2.95.2/mingw32 - see modified_pool.zip.
> 
> * mutex.h:
> 
> I think that mutex should be split into three separate classes,
> detail::win32_mutex, detail::pthread_mutex, and
> detail::single_threaded_mutex; mutex would be conditionally 
typedef'ed to
> one of them. This will deal more gracefully with inconsistent 
definitions.
> 
> Win9x doesn't have trylock on a critical section; perhaps 
mutex::trylock()
> ought to be removed?
> 
> Don't want to reopen an old debate but shouldn't lock/unlock be 
private and
> accessible via guard<> only?

I hope that the mutex is an implementation detail (in a detail 
namespace) that shouldn't be touched by the users of this library.  
I've not looked at the code to verify this, but I would be loathe to 
except any library that exposes such a concept publicly at this 
point.  The Boost Thread library will address all of these issues, 
and more, with a portable set of synchronization primitives that 
should replace the implementation details in any library accepted 
today.  It would be a bad idea, IMHO, to have multiple such concepts 
exposed publicly in Boost.

As for the try lock... if it's not used by the library remove it.  If 
it is, use a Win32 mutex instead of a critical section.  The Boost 
threading library will address performance (the current Win32 
implementation attempts to do this, and succeeds beautifully on some 
hardware/OS's while not on others, but this is something I expect to 
work out).

Bill Kempf