[Piglit] [PATCH 2/3] framework: allow specifying the number of jobs for concurrency

[Dylan Baker]

Quoting Marek Olšák (2018-05-30 13:04:47)
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> The default remains the same: number of CPUs. But on systems with lots of
> cores but comparatively little (V)RAM it can make sense to reduce the
> number of jobs to avoid random failures caused by out-of-memory conditions.
> ---
>  framework/options.py      |  1 +
>  framework/profile.py      |  7 +++++--
>  framework/programs/run.py | 23 +++++++++++++++++++++--
>  3 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/framework/options.py b/framework/options.py
> index 211159a45..b6ff2b406 100644
> --- a/framework/options.py
> +++ b/framework/options.py
> @@ -51,20 +51,21 @@ class _Options(object):  # pylint: disable=too-many-instance-attributes
>      env -- environment variables set for each test before run
>      deqp_mustpass -- True to enable the use of the deqp mustpass list feature.
>      """
>  
>      def __init__(self):
>          self.execute = True
>          self.valgrind = False
>          self.sync = False
>          self.deqp_mustpass = False
>          self.process_isolation = True
> +        self.jobs = -1

This should be None by default instead of -1.

>  
>          # env is used to set some base environment variables that are not going
>          # to change across runs, without sending them to os.environ which is
>          # fickle and easy to break
>          self.env = {
>              'PIGLIT_SOURCE_DIR':
>                  os.environ.get(
>                      'PIGLIT_SOURCE_DIR',
>                      os.path.abspath(os.path.join(os.path.dirname(__file__),
>                                                   '..')))
> diff --git a/framework/profile.py b/framework/profile.py
> index ffc91e0a6..cdd0d3057 100644
> --- a/framework/profile.py
> +++ b/framework/profile.py
> @@ -590,37 +590,38 @@ def load_test_profile(filename, python=None):
>                  filename))
>  
>      try:
>          return mod.profile
>      except AttributeError:
>          raise exceptions.PiglitFatalError(
>              'There is no "profile" attribute in module {}.\n'
>              'Did you specify the right file?'.format(filename))
>  
>  
> -def run(profiles, logger, backend, concurrency):
> +def run(profiles, logger, backend, concurrency, jobs=-1):

Don't give this a default value, just leave as jobs since it'll always be passed
(unless I missed something.)

>      """Runs all tests using Thread pool.
>  
>      When called this method will flatten out self.tests into self.test_list,
>      then will prepare a logger, and begin executing tests through it's Thread
>      pools.
>  
>      Based on the value of concurrency it will either run all the tests
>      concurrently, all serially, or first the thread safe tests then the
>      serial tests.
>  
>      Finally it will print a final summary of the tests.
>  
>      Arguments:
>      profiles -- a list of Profile instances.
>      logger   -- a log.LogManager instance.
>      backend  -- a results.Backend derived instance.
> +    jobs     -- maximum number of concurrent jobs. Use os.cpu_count() by default
>      """
>      chunksize = 1
>  
>      profiles = [(p, p.itertests()) for p in profiles]
>      log = LogManager(logger, sum(len(p) for p, _ in profiles))
>  
>      # check that after the filters are run there are actually tests to run.
>      # if not any(l for _, l in profiles):
>          # raise exceptions.PiglitUserError('no matching tests')
>  
> @@ -663,21 +664,23 @@ def run(profiles, logger, backend, concurrency):
>              # pool
>              run_threads(single, profile, test_list[1],
>                          lambda x: not x[1].run_concurrent)
>          profile.teardown()
>  
>      # 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()
> +    if not jobs or jobs < 0:
> +        jobs = os.cpu_count()
> +    multi = multiprocessing.dummy.Pool(jobs)

If you set processes=None instead of processes=-1 by default we can drop the if
statement above, when processes == None, os.cpu_count() is used automatically by
Pool class.

>  
>      try:
>          for p in profiles:
>              run_profile(*p)
>  
>          for pool in [single, multi]:
>              pool.close()
>              pool.join()
>      finally:
>          log.get().summary()
> diff --git a/framework/programs/run.py b/framework/programs/run.py
> index 14fb764a2..bbc527b55 100644
> --- a/framework/programs/run.py
> +++ b/framework/programs/run.py
> @@ -201,20 +201,28 @@ def _run_parser(input_):
>                          dest='process_isolation',
>                          action='store',
>                          type=booltype,
>                          default=core.PIGLIT_CONFIG.safe_get(
>                              'core', 'process isolation', 'true'),
>                          metavar='<bool>',
>                          help='Set this to allow tests to run without process '
>                               'isolation. This allows, but does not require, '
>                               'tests to run multiple tests per process. '
>                               'This value can also be set in piglit.conf.')
> +    parser.add_argument('-j', '--jobs',
> +                        dest='jobs',
> +                        action='store',
> +                        type=int,
> +                        default=core.PIGLIT_CONFIG.safe_get(
> +                            'core', 'jobs', '-1'),

None here as well.

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20180530/c455aed6/attachment-0001.sig>