[Piglit] [PATCH v2] framework/profile.py: raise exception from pool.

[Jose Fonseca]

On 07/12/15 20:25, baker.dylan.c at gmail.com wrote:
> From: Dylan Baker <baker.dylan.c at gmail.com>
>
> By using Multiprocessing.dummy.Pool.apply_async() instead of .imap(), an
> exception in the thread can be raised stopping the run of the suite.
>
> v2: - Actually run in parallel.
>
> Signed-off-by: Dylan Baker <dylanx.c.baker at intel.com>
>
> There is not a noticeable change in performance, and there is no change
> in results with this change.
> ---
>   framework/profile.py | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/framework/profile.py b/framework/profile.py
> index 32ed759..8643af6 100644
> --- a/framework/profile.py
> +++ b/framework/profile.py
> @@ -248,22 +248,33 @@ class TestProfile(object):
>
>           self._pre_run_hook()
>
> -        chunksize = 1
> -
>           self._prepare_test_list()
>           log = LogManager(logger, len(self.test_list))
>
> -        def test(pair):
> +        def test(name, test):
>               """Function to call test.execute from map"""
> -            name, test = pair
>               with backend.write_test(name) as w:
>                   test.execute(name, log.get(), self.dmesg)
>                   w(test.result)
>
>           def run_threads(pool, testlist):
> -            """ Open a pool, close it, and join it """
> -            pool.imap(test, testlist, chunksize)
> +            """Run all of the tests, and look for exceptions.
> +
> +            This creates a list of results, and then checks them. the only
> +            reason to check each result is that if there is an exception then
> +            it will be raised in the main thread rather than in the worker
> +            threads.
> +
> +            """
> +            results = []
> +            for pair in testlist:
> +                results.append(pool.apply_async(test, pair))

I'm surprised this makes a difference.  I'd imagine that imap would 
behave the same way.

Regardless of this change, I wonder if we should have a 
catch-all-exception right at the top of test() function, that dumps the 
exception and aborts.  Just in case the internals of multiprocessing 
change again.

Eitherway,

Reviewed-by: Jose Fonseca <jfonseca at vmware.com>

> +
>               pool.close()
> +
> +            for r in results:
> +                r.get()
> +
>               pool.join()
>
>           # Multiprocessing.dummy is a wrapper around Threading that provides a
>


Jose