Re: Bug 324228 – support threadpool exiting idle threads

[Sebastian Wilhelmi <seppi seppi de>]

Hi,

On Fri, 2005-12-23 at 12:24 -0500, Matthias Clasen wrote: 
> On Fri, 2005-12-23 at 14:53 +0100, Tim Janik wrote:
> > On Fri, 23 Dec 2005, Martyn Russell wrote:
> > 
> > > Guys,
> > >
> > > I am keen to get this thread pool patch committed to CVS before the GLib
> > > API freeze over Christmas. If it could be reviewed so this would be
> > > possible I would appreciate it. The bug report associated with this
> > > patch is here:
> > >
> > > http://bugzilla.gnome.org/show_bug.cgi?id=324228
> > >
> > > A few people have already looked at it and I have comprehensively
> > > updated the tests/threadpool-test app so people can test the new sorting
> > > (which was committed recently) and the idle thread patch.
> > >
> > > The reason for this work is explained in the bug and it related to
> > > GnomeVFS.
> > 
> > sebastian,
> > 
> > is there any chance you can review this in the next few days?
> > if not, i think we should get this in before the API freeze
> > anyway (the API looks ok to me now) and can fixup implementation
> > details after that.

I think, the API is ok now - it could go in. I want to change the
current implementation, however. This is actually necessary
independently of the max-idle-time patch. See below.

> > matthias, what's your take on this wrg the freeze?
> 
> For the idle timeout patch, I think we need to understand the desired
> behaviour a bit better, before rushing it in. My understanding of the
> current threadpool behaviour is the following:
> 
> 1) there is a global pool of idle threads, which can be restricted in
>   size by setting max-idle-threads to some non-negative number
> 
> 2) threadpools take idle threads from the global pool to do their work
> 
> 3) if a threadpool has no more tasks to give to a thread, the thread is
>   moved back to the global idle threads pool after a delay of 0.5s

Yes, that sounds like, what I would expect. However, looking at the
code, it seems that threads remain attached to their thread pool and
only go the global pool after the maximal number of threads for the pool
is overstepped. Really, I have no idea, why I did that in the first
place. I hope, it made sense to me then ;-)

> Is the idea to make the delay in 3) changeable, or is the idea to
> introduce a second timeout that would cause threads to be removed from
> the global pool ? 

I think, what is planned and what really makes sense is the second, i.e.
making the time, which an idle threads stays in the global pool,
configurable. The half second is just an implementation optimization and
shouldn't matter much.

> Would this only kick in if all threadpools are idle ?

It shouldn't be dependent on the state of any thread pool, only on the
time of the individual threads in the global pool. 

> Should there be a min-idle-threads, to guarantee that at least some
> threads stay in the global pool, even if there is no activity for a long
> time ?

Maybe, I don't know.

> It might be good to port gnome-vfs to gthreadpool now (that should be
> already possible, since the sorting api went in), and see if we actually
> need any extra tweaks.

I think, it doesn't hurt to add the two functions to the API.

I hope to come up with a better version of the thread pools over the
holidays.

Merry Christmas,
Sebastian.
-- 
Sebastian Wilhelmi                 |            här ovanför alla molnen
mailto:seppi seppi de              |      är himlen så förunderligt blå
http://seppi.de