[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