API Proposal for apr_thread_pool

[Henry Jen <he...@ztune.net>]

Hi,

We would like to implement thread pool capability, and think it might be 
a good addition to apr-util or apr.

I have defined a header file and is going to implement it, if you can 
review it and give some feedback, it is much appreciated.

Cheers,
Henry

Re: API Proposal for apr_thread_pool

[Henry Jen <he...@ztune.net>]

Henry Jen wrote:
> William A. Rowe, Jr. wrote:
>> Henry Jen wrote:
> 
>>> /**
>>>  * Stop all unused threads. Ignore the maximum number of idling threads.
>>>  * @return The total number of threads stopped.
>>>  */
>>> APR_DECLARE(int) apr_thread_pool_stop_unused_threads(void);
>>
>> I'm a little confused in your proposal between 
>> apr_thread_pool_terminate(),
>> apr_thread_pool_destroy(), and this function.
>>
>> Can't this be reduced to
>>
> 
> My mistake. The last couple functions should take a apr_thread_pool_t 
> *self as a parameter. Those are instance methods, not static.
> 

Another thing is that the initialize/terminate may not be needed. I put 
it there for now in case some thing I have overlooked.

Cheers,
Henry

Re: API Proposal for apr_thread_pool

[Henry Jen <he...@ztune.net>]

William A. Rowe, Jr. wrote:
> Henry Jen wrote:
>> Hi,
>>
>> We would like to implement thread pool capability, and think it might 
>> be a good addition to apr-util or apr.
> 
> Interesting!
> 
> I don't see where your thread join occurs to reap any terminated threads,
> that is, how your model handles threads that have decided they are 'done'.
>

The thread remains in the pool until the thread_pool tells the thread to 
stop and that's where join will happen.

One place is apr_thread_pool_stop_unused_threads.

>> APR_DECLARE(int) apr_thread_pool_get_num_unused_threads(void);
> 
> AIUI the style apr_thread_pool_unused_threads_num_get() would be 
> appropriate
> (_num seems awkward though, I personally prefer _count, or nothing).
> 

I am OK with the naming convention as long as there is one. :-)

>> /**
>>  * Stop all unused threads. Ignore the maximum number of idling threads.
>>  * @return The total number of threads stopped.
>>  */
>> APR_DECLARE(int) apr_thread_pool_stop_unused_threads(void);
> 
> I'm a little confused in your proposal between apr_thread_pool_terminate(),
> apr_thread_pool_destroy(), and this function.
> 
> Can't this be reduced to
> 

My mistake. The last couple functions should take a apr_thread_pool_t 
*self as a parameter. Those are instance methods, not static.

This may address all the following questions.

Attached please find updated version.

Cheers,
Henry

>   apr_thread_pool_shutdown()  (blocks)
>   apr_thread_pool_destroy()   (same as pool cleanup)
> 
> Can you elaborate?
> 
> Final observation, your model seemed a little shortsighted, in that it 
> permits
> only a single thread pool.  This is great for an app, lousy for another 
> library
> which wishes to build upon the model.  And a group of threads might be 
> shut down
> independently of a second pool.
> 
> Can you refine the proposal with an apr_threadpool_t * object which 
> prevents
> us from adding yet another evil static singleton?  Or as an alternative, 
> bind
> the threadpool as an attribute of the pool itself, identifying the 
> thread pool
> by apr_pool_t *?
> 
> Bill
> 

Re: API Proposal for apr_thread_pool

[Garrett Rooney <ro...@electricjellyfish.net>]

On 4/5/06, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:

> Final observation, your model seemed a little shortsighted, in that it permits
> only a single thread pool.  This is great for an app, lousy for another library
> which wishes to build upon the model.  And a group of threads might be shut down
> independently of a second pool.
>
> Can you refine the proposal with an apr_threadpool_t * object which prevents
> us from adding yet another evil static singleton?  Or as an alternative, bind
> the threadpool as an attribute of the pool itself, identifying the thread pool
> by apr_pool_t *?

I'd prefer a separate threadpool object, but yes, I agree that the API
needs to allow for multiple pools in an application.

-garrett

Re: API Proposal for apr_thread_pool

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Henry Jen wrote:
> Hi,
> 
> We would like to implement thread pool capability, and think it might be 
> a good addition to apr-util or apr.

Interesting!

I don't see where your thread join occurs to reap any terminated threads,
that is, how your model handles threads that have decided they are 'done'.

> APR_DECLARE(int) apr_thread_pool_get_num_unused_threads(void);

AIUI the style apr_thread_pool_unused_threads_num_get() would be appropriate
(_num seems awkward though, I personally prefer _count, or nothing).

> /**
>  * Stop all unused threads. Ignore the maximum number of idling threads.
>  * @return The total number of threads stopped.
>  */
> APR_DECLARE(int) apr_thread_pool_stop_unused_threads(void);

I'm a little confused in your proposal between apr_thread_pool_terminate(),
apr_thread_pool_destroy(), and this function.

Can't this be reduced to

   apr_thread_pool_shutdown()  (blocks)
   apr_thread_pool_destroy()   (same as pool cleanup)

Can you elaborate?

Final observation, your model seemed a little shortsighted, in that it permits
only a single thread pool.  This is great for an app, lousy for another library
which wishes to build upon the model.  And a group of threads might be shut down
independently of a second pool.

Can you refine the proposal with an apr_threadpool_t * object which prevents
us from adding yet another evil static singleton?  Or as an alternative, bind
the threadpool as an attribute of the pool itself, identifying the thread pool
by apr_pool_t *?

Bill