apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: Thread pool prototype
Date Mon, 01 May 2006 11:23:48 GMT
Henry Jen wrote:
> 
> Please let me know if you have any comments. :-)
> +/**
> + * Setup all of the internal structures required to use thread pools
> + */
> +APR_DECLARE(apr_status_t) apr_thread_pool_init(void);
> +
> +/**
> + * Tear down all of the internal structures required to use pools
> + */
> +APR_DECLARE(void) apr_thread_pool_terminate(void);

Double initialization, double teardown issues?  Consider that my libfoo.so
may want to create a threadpool, I'm not privy to whether or not the application
that embedded libfoo.so actualy intialized the threadpool API itself.

Is this needed?  If not can we please chuck it?  I understand the desire to
anticipate the unanticipated, but this certainly will be a bigger problem if
it were used, than if we discover it's needed somehow.

> +APR_DECLARE(apr_thread_pool_t*) apr_thread_pool_create(apr_pool_t *pool,
> +                                                       apr_size_t init_threads, 
> +                                                       apr_size_t max_threads, 
> +                                                       apr_status_t *err);

EWWWW!

Do you mean

APR_DECLARE(apr_status_t) apr_thread_pool_create(apr_thread_pool_t **threadpool,
                                                  apr_pool_t *pool,
                                                  apr_size_t init_threads,
                                                  apr_size_t max_threads);

Please follow style guidelines, and precident, in your proposals :)

> +/**
> + * Get current number of tasks waiting in the queue
> + * @param me The thread pool
> + * @return Number of tasks in the queue
> + */
> +APR_DECLARE(int) apr_thread_pool_tasks_cnt_get(apr_thread_pool_t *me);

Worst of all worlds.  It's not a property (e.g. get/set), it's a verb (please
*count* for me your entities), so apr_thread_pool_tasks_count( ) seems right.

> +/**
> + * Get current number of idling thread
> + * @param me The thread pool
> + * @return Number of idling threads
> + */
> +APR_DECLARE(int) apr_thread_pool_unused_cnt_get(apr_thread_pool_t *me);

ditto... _count() not cnt_get()

> +/**
> + * Stop all unused threads. Ignore the maximum number of idling threads.
> + * @param me The thread pool
> + * @return The total number of threads stopped.
> + */
> +APR_DECLARE(int) apr_thread_pool_stop_unused_threads(apr_thread_pool_t *me);

What's the difference between setting apr_thread_pool_unused_max_set() to 0,
and this function?  Can we drop it?

It seems this function would stop idling threads at this point in time, while
it makes much more design sense to set unused_max to 0, ensuring the unused
threads continue to be pruned as they finish their unit of work.

Thoughts?

Bill

Mime
View raw message