[Piglit] [PATCH] profile: Fix mixed concurrency runs

[Ilia Mirkin]

On Thu, Apr 17, 2014 at 4:59 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> Currently we call join after initializing both of the pools, which means
> that they run simultaneously. This patch fixes that by creating a helper
> function which sets off the pool, closes it, and then joins it. This
> fixes the problem by forcing each pool to run in series.
>
> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> ---
>  framework/profile.py | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/framework/profile.py b/framework/profile.py
> index 2e160e3..3def3e0 100644
> --- a/framework/profile.py
> +++ b/framework/profile.py
> @@ -119,6 +119,8 @@ class TestProfile(object):
>          self.prepare_test_list(env)
>          log = Log(len(self.test_list), env.verbose)
>
> +        chunksize = 1
> +
>          def test(pair):
>              """ Function to call test.execute from .map
>
> @@ -128,33 +130,30 @@ class TestProfile(object):
>              name, test = pair
>              test.execute(name, log, json_writer, self.dmesg)
>
> +        def run_threads(pool, testlist):
> +            """ Open a pool, close it, and join it """
> +            pool.imap(test, testlist, chunksize)
> +            pool.close()
> +            pool.join()
> +
>          # Multiprocessing.dummy is a wrapper around Threading that provides a
>          # multiprocessing compatible API
>          #
>          # The default value of pool is the number of virtual processor cores
>          single = multiprocessing.dummy.Pool(1)
>          multi = multiprocessing.dummy.Pool()
> -        chunksize = 1
>
>          if env.concurrent == "all":
> -            multi.imap(test, self.test_list.iteritems(), chunksize)
> +            run_threads(multi, self.test_list.iteritems())
>          elif env.concurrent == "none":
> -            single.imap(test, self.test_list.iteritems(), chunksize)
> +            run_threads(single, self.test_list.iteritems())
>          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].run_concurrent), chunksize)
> +            run_threads(multi, (x for x in self.test_list.iteritems() if
> +                                x[1].run_concurrent))
>              # 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].run_concurrent), chunksize)
> -
> -        # 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()
> -        multi.join()
> -        single.join()
> +            run_threads(multi, (x for x in self.test_list.iteritems() if not
> +                                x[1].run_concurrent))

At least one of these should be single, no? Otherwise the
non-concurrent tests will get run in the multi threadpool. Also, IMO
placing "if not" on the next line, next to the condition (and aligned
with the opening paren of the generator) would make it a lot more
readable.

>
>          log.summary()
>
> --
> 1.9.2
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit