[Piglit] [PATCH] framework: avoid blocking joins, allowing ^C to work

[Daniel Vetter]

On Sun, Feb 16, 2014 at 06:15:40PM -0500, Ilia Mirkin wrote:
> On Sun, Feb 16, 2014 at 6:14 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > On 02/15/2014 10:30 AM, Ilia Mirkin wrote:
> >> On Sat, Feb 15, 2014 at 7:37 AM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> >>> On Saturday, February 15, 2014 03:27:44 AM Ilia Mirkin wrote:
> >>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> >>>> ---
> >>>>  framework/core.py | 20 ++++++++++++++------
> >>>>  1 file changed, 14 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/framework/core.py b/framework/core.py
> >>>> index 4bcaa82..7b2083b 100644
> >>>> --- a/framework/core.py
> >>>> +++ b/framework/core.py
> >>>> @@ -31,6 +31,7 @@ import sys
> >>>>  import time
> >>>>  import traceback
> >>>>  from cStringIO import StringIO
> >>>> +import itertools
> >>>>  import multiprocessing
> >>>>  import multiprocessing.dummy
> >>>>  import importlib
> >>>> @@ -578,22 +579,29 @@ class TestProfile(object):
> >>>>          chunksize = 1
> >>>>
> >>>>          if env.concurrent == "all":
> >>>> -            multi.imap(test, self.test_list.iteritems(), chunksize)
> >>>> +            testiter = multi.imap(test, self.test_list.iteritems(),
> >>>> chunksize) elif env.concurrent == "none":
> >>>> -            single.imap(test, self.test_list.iteritems(), chunksize)
> >>>> +            testiter = single.imap(test, self.test_list.iteritems(),
> >>>> chunksize) else:
> >>>>              # Filter and return only thread safe tests to the threaded pool
> >>>> -            multi.imap(test, (x for x in self.test_list.iteritems() if -
> >>>>                            x[1].runConcurrent), chunksize)
> >>>> +            iter1 = multi.imap(test, (x for x in self.test_list.iteritems()
> >>>> +                                      if x[1].runConcurrent), chunksize) #
> >>>> Filter and return the non thread safe tests to the single pool -
> >>>> single.imap(test, (x for x in self.test_list.iteritems() if not -
> >>>>                     x[1].runConcurrent), chunksize)
> >>>> +            iter2 = single.imap(test, (x for x in
> >>>> self.test_list.iteritems() +                                       if not
> >>>> x[1].runConcurrent), chunksize) +            testiter =
> >>>> itertools.chain(iter1, iter2)
> >>>>
> >>>>          # Close and join the pools
> >>>>          # If we don't close and the join the pools the script will exit
> >>>> before # the pools finish running
> >>>>          multi.close()
> >>>>          single.close()
> >>>> +
> >>>> +        # This waits for all the results to arrive in a non-blocking
> >>>> +        # fashion. This in turn allows ^C to work to stop a piglit run.
> >>>> +        for test in testiter:
> >>>> +            pass
> >>>> +
> >>>>          multi.join()
> >>>>          single.join()
> >>>
> >>> The code is valid and works as advertised. I actually like what's happening
> >>> now since ctrl-c kills the currently running test, and can be used to kill
> >>> stuck tests, but if other like the previous behavior better, it isn't a big
> >>> deal to me either way.
> >>>
> >>> Reviewed-by: Dylan Baker <baker.dylan.c at gmail.com>
> >>
> >> Hmmmm... I hadn't thought about that. It's just really hard to kill
> >> piglit right now -- you have to ^Z + kill -9 %%. How does using ^C in
> >> a parallel run (which I understand is the majority use-case) possible
> >> to kill the 'current' test -- there is no current test.
> >>
> >> Anyone else with opinions on what ^C should do while running piglit?
> >>
> >>   -ilia
> >
> > In the past, people have objected to Ctrl+C killing whole runs.  That
> > might be different if resume support was more reliable...I don't know.
> >
> > However, with concurrency, it's really not obvious what it does.  Some
> > test gets victimized...or sometimes the runner.  It's been discussed,
> > and there were no good answers.
> >
> > Ctrl+\ sends SIGQUIT and that always kills the whole thing.  It's just
> > not as well known.
> 
> Ah, yes, I did not know that.
> 
> >
> > I suspect if you change the behavior of Ctrl+C people will come
> > complain.  I don't know that it's justified, really, but...
> 
> OK. I had assumed that it was accidental that ^C didn't work. If it's
> well-thought-out (or at least well-understood/expected) behaviour,
> happy to drop the patch.

For the record I use ^C quite a bit for running igt, especially now that
the timeout stuff is gone again. We have much shittier tests than anyone
else though ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch