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 20:33:42 GMT
Henry Jen wrote:
> William A. Rowe, Jr. wrote:
>> 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 :)
> Is there a guideline other than 
> http://httpd.apache.org/dev/styleguide.html?

Not AFAIK, except that the existing code serves as a style guide, this is true
in all open source projects even without published formal style guides.

> I understand the precedent, but I thought there is no requirement. :-D
> apr_hash_t *     apr_hash_make (apr_pool_t *pool)

You choose a really bad example, apr_hash_make has no apr_status_t result.

> I prefer this way because it is clear that an app can check against the 
> returned pointer to be NULL without worrying the value to be undefined 
> when an error occurs.

Checking the returned pointer is a larger and ergo more inefficent compare
than the apr_status_t on 64LP platforms.

> That said, I will fix it.

Thanks :)

>> ditto... _count() not cnt_get()
> Thank you! Pardon me as not a native English speaker.

No problem :)  The more irritating bit was the abbrievation cnt - when the
rest was spelled out, but it seems to be the _get bit was excessive :)

> In a way they work the same. But there are a few interesting differences:
> 1. Set the unused max does not force the thread to stop, an unused 
> thread kills itself when it becomes idle and realize there are enough 
> idle threads. Thus no joining or any locking issue for using this function.

Is it reasonable to do a quick reap when max() is adjusted?  E.g. if the
freecount > max then call reap?

> 2. stop_unused_threads kills *all* unused threads regardless the 
> unused_max.

That's interesting - what purpose would this 'feature' serve?

> Is that mechanism make sense or just adding complexity? Not sure. It is 
> for occasions you want to kill all unused threads and allow them to grow 
> later. But why does an app do that? Under a resource constraint 
> environment, this can serve as a mechanism for error recovery. Of 
> course, you can do that with two consequent call to set_unused_max.

Once you've hit the wall, bits like this stand no chance of fixing things.
Apache HTTP and APR have longstanding policies of simply bailing once
allocations have started to fail altogether.

> Think of that, I forgot to signal the idle threads when new tasks are 
> added! :-P

Whoops!  :)

What would you think in general about changing 'unused' threads to 'free'
threads throughout the comments/implementation?

Anyways, I ment to add, nice work thus far.  If you have yet to do so,
please submit your CLA to our secretary Jim so that your implementation
can be considered for commit.  (We are not picking about a few-line patch
here and there, but for major works, it's necessary.)  Find it here;

I'm happy to commit this to a sandbox or trunk for now to let everyone
begin iteratively improving it, for adoption with a 1.3, 1.4 or 2.0 APR
release, once we think it's baked.  Just need the CLA first, and your
final draft (I'd hold off just a bit for feedback from more of the
peanut gallery.)



View raw message