76198 - Threadpool optimistically kills worker threads

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Doug Turner (:dougt)]

SUMMARY

The threadpool optimistically kills worker threads as soon as the total number 
of threads is greater than the minimun limit.  

The file transport uses the thread pool for its worker threads.  During startup, 
there is an exceptional ammount of file transport activity.  For every file that 
is loaded in, there must exist a worker thread in the threadpool.  The file 
transport sets min and max number for the number of threads.  The current logic 
in the threadpool is that it will spawn new thread for all requests up to the 
limit. When the worker thread, when finished, will check to see if there are any 
pending requests.  If there are none and there are more threads in the 
threadpool than the set lower limit, the thread immeditely exits.  

Because of this algorithm, there is alot of turnover in that pool.  The file 
transport could just raise the lower limit so that there are more persistant 
threads, however, this would just add to bloat since these extra thread are 
primarly needed for startup.

Another, better, solution would be to include a delay so that we can catch any 
additional request that may come in.  

For example:

From a normal startup to ftp://ftp.netscape.com.org, there are 73 new threads 
created and 72 threads deleted.  (This is completely insane) After putting a 5 
second wait, there are only 13 new threads created and 12 deleted.  This removes 
60 thread creation/deletions.  

Going to http://www.mozilla.org creates 49 threads.  Adding a 5 second delay, 
reduces this to creating only 6 threads.

Clearly, there should be some period of wait time before the thread exist so 
that we don't get a huge number of thread exits followed by an immeditate start.

I am not sure exactly what this will trim off of startup time.  I ran some basic 
timing tests.  On a P3 dual 500 (windows 2k, 128mb ram), it take only 2ms to 
create 10 threads. However, this is not a accurate picture since creation of a 
thread doesn't really do much but schedules it.  I also looked at how long it 
takes from the creation of a thread to its actual running.  The data indicates 
that it takes longer as there are more threads.  With 3 threads present, it 
takes on average 200 milliseconds from the time PR_CreateThread() is called to 
the time the user defined function is entered and does a printf().  Maybe 
somebody here has better knowledge of thread startup preformance.

Attached in my proposed change.

Comment 1

[Doug Turner (:dougt)]

Attachment: Proposed Fix v1

Comment 2

[Doug Turner (:dougt)]

Setting Milestone for 0.9.  I am going to need some reviewers here.  

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

At the moment, on linux at least, gdb only supports 32 threads, so this is
triggering gdb barfage too.

Even with your fix 13 threads created and 12 deleted seems quite high.  Any way
to get it down even further?  Note that thread creation may be significantly
more expensive depending on the OS it's happening on.

Comment 4

[Doug Turner (:dougt)]

the three ways to get the total number of threads down even further is either to 
increase the wait period, or decrease the number of threads, or some combo of 
these two.

Comment 5

[Chris Waterson]

r=waterson

Comment 6

[Doug Turner (:dougt)]

setting milestone...

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

How about making the delay 10 or even 30 seconds?  

What are the current low and high thread limits?

Comment 8

[Doug Turner (:dougt)]

the limits are set by the users of the threadpool.  The only user that I am 
aware of is the file transport.  It has chosen 1 and 4 as the lower and 
upper limits respectively.  

Increasing this number to 10 or 15 may be wise.  

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Maybe a minimum of 3 or 4 and a max of 5-10?

Comment 10

[Doug Turner (:dougt)]

has been review by waterson and darin.  This should improve startup time. 

Comment 11

[Doug Turner (:dougt)]

darin suggests that I increase the timeout value to 30 seconds.

Comment 12

[Doug Turner (:dougt)]

No raising the minimum.  We don't want thread bloat for the sake of thread 
bloat.  Increasing the time before the worker thread goes away is enough.

Comment 13

[Asa Dotzler [:asa]]

Are you still shooting for 0.9 on this? If so please email [e-mail]
with a status on you progress. If not please retarget against a later Milestone.
Thanks.

Comment 14

[Doug Turner (:dougt)]

I will check this in after the tree opens for 0.9.1

Comment 15

[Doug Turner (:dougt)]

Fix checked in.  Tomorrows startup numbers should be very interesting.  

Paul, can you take some measurements between 0.9 and tomorrows 0.9.1?