Re: Bug 324228 – support threadpool exiting idle threads

[Matthias Clasen <mclasen redhat com>]

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.
> 
> 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


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 ? Would this only kick in if all threadpools are idle ?
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 ?

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.

Matthias