[libcxx-commits] [libcxx] ff09135 - [libc++] Execute tests from the Lit execution root instead of the test tree

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Wed Apr 1 22:11:47 PDT 2020


Yes, I did, and the difference appears to be in the noise. On my machine:

Without temporary directories
-----------------------------

    $ time ninja -C build check-cxx  

    Testing Time: 291.68s
    Expected Passes    : 6034
    Expected Failures  : 34
    Unsupported Tests  : 360

    3980.19s user 703.50s system 1591% cpu 4:54.21 total


With temporary directories
--------------------------

    $ time ninja -C build check-cxx

    Testing Time: 293.28s
    Expected Passes    : 6034
    Expected Failures  : 34
    Unsupported Tests  : 360

    3986.36s user 708.69s system 1588% cpu 4:55.62 total


This is weird, as creating 6000 temporary directories has a lot more than a 1 second overhead (tried!), but I guess that overhead is somehow buried in the context of the whole test suite.

Note that I would love to explore ways to speed up the execution of the test suite, but for now I'm focusing on making it run correctly everywhere -- so long as there's no big speed regression of course.

Louis

> On Apr 1, 2020, at 22:19, Nico Weber <thakis at chromium.org> wrote:
> 
> Did you check if this slows down test execution at all? Creating a temp dir per test sounds potentially slow.
> 
> On Wed, Apr 1, 2020 at 10:17 PM Louis Dionne via libcxx-commits <libcxx-commits at lists.llvm.org <mailto:libcxx-commits at lists.llvm.org>> wrote:
> 
> Author: Louis Dionne
> Date: 2020-04-01T22:17:03-04:00
> New Revision: ff09135fc2b7a9696f87a8a8e89be2ef777895d3
> 
> URL: https://github.com/llvm/llvm-project/commit/ff09135fc2b7a9696f87a8a8e89be2ef777895d3 <https://github.com/llvm/llvm-project/commit/ff09135fc2b7a9696f87a8a8e89be2ef777895d3>
> DIFF: https://github.com/llvm/llvm-project/commit/ff09135fc2b7a9696f87a8a8e89be2ef777895d3.diff <https://github.com/llvm/llvm-project/commit/ff09135fc2b7a9696f87a8a8e89be2ef777895d3.diff>
> 
> LOG: [libc++] Execute tests from the Lit execution root instead of the test tree
> 
> Instead of executing tests from within the libc++ test suite, we execute
> them from the Lit execution directory. However, since some tests have
> file dependencies, we must copy those dependencies to the execution
> directory where they are executed.
> 
> This has the major benefit that if a test modifies a file (whether it
> is wanted or not), other tests will not see those modifications. This
> is good because current tests assume that input data is never modified,
> however this could be an incorrect assumption if some test does not
> behave properly.
> 
> Added: 
> 
> 
> Modified: 
>     libcxx/utils/libcxx/test/config.py
>     libcxx/utils/libcxx/test/executor.py
>     libcxx/utils/libcxx/test/format.py
>     libcxx/utils/run.py
> 
> Removed: 
> 
> 
> 
> ################################################################################
> diff  --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
> index 3e983e1b4d4a..20d0a796a3af 100644
> --- a/libcxx/utils/libcxx/test/config.py
> +++ b/libcxx/utils/libcxx/test/config.py
> @@ -1050,7 +1050,6 @@ def configure_substitutions(self):
>              exec_args.append('--host {}'.format(self.executor.user_prefix + self.executor.host))
>              executor = os.path.join(self.libcxx_src_root, 'utils', 'ssh.py')
>          else:
> -            exec_args.append('--working_directory "%S"')
>              executor = os.path.join(self.libcxx_src_root, 'utils', 'run.py')
>          sub.append(('%{exec}', '{} {} {} -- '.format(pipes.quote(sys.executable),
>                                                       pipes.quote(executor),
> 
> diff  --git a/libcxx/utils/libcxx/test/executor.py b/libcxx/utils/libcxx/test/executor.py
> index b555b1f03df9..c34310cdd2e2 100644
> --- a/libcxx/utils/libcxx/test/executor.py
> +++ b/libcxx/utils/libcxx/test/executor.py
> @@ -10,6 +10,7 @@
>  import os
>  import posixpath
>  import ntpath
> +import shutil
> 
>  from libcxx.test import tracing
>  from libcxx.util import executeCommand
> @@ -61,6 +62,12 @@ def run(self, exe_path, cmd=None, work_dir='.', file_deps=None, env=None):
>          if env:
>              env = self.merge_environments(os.environ, env)
> 
> +        for dep in file_deps:
> +            if os.path.isdir(dep):
> +                shutil.copytree(dep, os.path.join(work_dir, os.path.basename(dep)), symlinks=True)
> +            else:
> +                shutil.copy2(dep, work_dir)
> +
>          out, err, rc = executeCommand(cmd, cwd=work_dir, env=env)
>          return (cmd, out, err, rc)
> 
> 
> diff  --git a/libcxx/utils/libcxx/test/format.py b/libcxx/utils/libcxx/test/format.py
> index a026c4f1ba02..1bc85f24976a 100644
> --- a/libcxx/utils/libcxx/test/format.py
> +++ b/libcxx/utils/libcxx/test/format.py
> @@ -9,6 +9,8 @@
>  import copy
>  import errno
>  import os
> +import shutil
> +import tempfile
>  import time
>  import random
> 
> @@ -209,16 +211,21 @@ def _evaluate_pass_test(self, test, tmpBase, lit_config,
>                  report += "Compilation failed unexpectedly!"
>                  return lit.Test.Result(lit.Test.FAIL, report)
>              # Run the test
> -            local_cwd = os.path.dirname(source_path)
>              env = None
>              if self.exec_env:
>                  env = self.exec_env
> 
>              max_retry = test.allowed_retries + 1
>              for retry_count in range(max_retry):
> -                cmd, out, err, rc = self.executor.run(exec_path, [exec_path],
> -                                                      local_cwd, data_files,
> -                                                      env)
> +                # Create a temporary directory just for that test and run the
> +                # test in that directory
> +                try:
> +                    execDirTmp = tempfile.mkdtemp(dir=execDir)
> +                    cmd, out, err, rc = self.executor.run(exec_path, [exec_path],
> +                                                          execDirTmp, data_files,
> +                                                          env)
> +                finally:
> +                    shutil.rmtree(execDirTmp)
>                  report = "Compiled With: '%s'\n" % ' '.join(compile_cmd)
>                  report += libcxx.util.makeReport(cmd, out, err, rc)
>                  if rc == 0:
> 
> diff  --git a/libcxx/utils/run.py b/libcxx/utils/run.py
> index 6a89a2b9388a..7de82c78dbfa 100644
> --- a/libcxx/utils/run.py
> +++ b/libcxx/utils/run.py
> @@ -14,14 +14,15 @@
> 
>  import argparse
>  import os
> +import shutil
>  import subprocess
>  import sys
> +import tempfile
> 
> 
>  def main():
>      parser = argparse.ArgumentParser()
>      parser.add_argument('--codesign_identity', type=str, required=False)
> -    parser.add_argument('--working_directory', type=str, required=True)
>      parser.add_argument('--dependencies', type=str, nargs='*', required=True)
>      parser.add_argument('--env', type=str, nargs='*', required=True)
>      (args, remaining) = parser.parse_known_args(sys.argv[1:])
> @@ -42,14 +43,23 @@ def main():
>      # Extract environment variables into a dictionary
>      env = {k : v  for (k, v) in map(lambda s: s.split('=', 1), args.env)}
> 
> -    # Ensure the file dependencies exist
> -    for file in args.dependencies:
> -        if not os.path.exists(file):
> -            sys.stderr.write('Missing file {} marked as a dependency of a test'.format(file))
> -            exit(1)
> +    try:
> +        tmpDir = tempfile.mkdtemp()
> 
> -    # Run the executable with the given environment in the given working directory
> -    return subprocess.call(' '.join(remaining), cwd=args.working_directory, env=env, shell=True)
> +        # Ensure the file dependencies exist and copy them to a temporary directory.
> +        for dep in args.dependencies:
> +            if not os.path.exists(dep):
> +                sys.stderr.write('Missing file or directory "{}" marked as a dependency of a test'.format(dep))
> +                exit(1)
> +            if os.path.isdir(dep):
> +                shutil.copytree(dep, os.path.join(tmpDir, os.path.basename(dep)), symlinks=True)
> +            else:
> +                shutil.copy2(dep, tmpDir)
> +
> +        # Run the executable with the given environment in the temporary directory.
> +        return subprocess.call(' '.join(remaining), cwd=tmpDir, env=env, shell=True)
> +    finally:
> +        shutil.rmtree(tmpDir)
> 
>  if __name__ == '__main__':
>      exit(main())
> 
> 
> 
> _______________________________________________
> libcxx-commits mailing list
> libcxx-commits at lists.llvm.org <mailto:libcxx-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/libcxx-commits>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200402/eb1d7db5/attachment.html>


More information about the libcxx-commits mailing list