[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: Thread pool efficiency

Howard Chu writes:
> True. Well as Rick suggested, we don't have to go to the extreme of
> one queue per thread, we can go part way and use one queue per N
> threads.

Sounds OK.  But I know little about scheduling, so I guess I'll mostly
stay out of that and your suggestion below for now.

>> Or with operations like adding a member to a 20000-member posixGroup.
>> I just did that, it took 0.7 seconds with back-bdb.
> Did you try that with sorted values?

Argh, I knew we'd forgotten something when we set up that new server...
Thanks for the tip.

> (...)
>> BTW, is the problem that each operation locks pool->ltp_mutex 2-3 times,
>> or is it the amount of time it is kept locked?  Most tests which
>> pool_<submit/wrapper>() do with ltp_mutex locked, could be precomputed.
> Both I think.

Then I'll at least reduce pool_wrapper a bit.  The for(;;) can become:
    for (;;) {
        task = LDAP_STAILQ_FIRST(pool->ltp_pending_listptr);
        if (task == NULL) {
            if (pool->ltp_close_thread)
                break; /* !ltp_pause && (FINISHING or too many threads) */
            ldap_pvt_thread_cond_wait(&pool->ltp_cond, &pool->ltp_mutex);
        <rest of loop untouched>;
ltp_pending_listptr == (ltp_pause ? &(empty list) : &ltp_pending_list).
Removed state STOPPING.  We can use FINISHING and flush pending_list.

Reducing _submit() gets a bit uglier.  The if (...RUNNING etc ...)
test and the "create new thread?" test can both be reduced to simple
compares, and the ltp_pause test can move into the branch for the
latter.  I think that'll make the file harder to rearrange later though,
so maybe it should wait.

> I haven't set up Dtrace on the T5120 yet to get better
> info, but oprofile on Linux shows that pthread_mutex_lock is taking
> too much CPU time (vying with the ethernet driver for #1
> consumer).  And realistically, a single shared resource like this is
> just a really bad idea.

True enough.  Still, slapd has a lot of mutexes.  Should perhaps check
if this one stands out before rearraning scheduling around it.