aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/test/shell/testenv.sh
diff options
context:
space:
mode:
authorGravatar Philipp Wollermann <philipp.wollermann@gmail.com>2018-03-30 14:39:54 -0700
committerGravatar Copybara-Service <copybara-piper@google.com>2018-03-30 14:41:41 -0700
commitb5e893a51cff0f555ed56d05193a32630b09fb89 (patch)
tree932ce845a0761369166facb4bccf3b8bdc1d981d /src/test/shell/testenv.sh
parent7a5761ca0b745c2a4085468d1262093164bcc1ad (diff)
Improve performance and output of bazel_determinism_test.
Don't ask me how so many things can be wrong in a single test... Progress towards #4770. FYI @rupertks @buchgr @ulfjack ## Replace 25000 invocations of perl with a single "sha256sum" This speeds up the test by a factor 2x on my iMac (before: 1200s, now: 600s). On macOS, "shasum" is a Perl script. Instead of simply passing all input files to the thing at once, we were invoking it once per file. This means roughly 25,000 invocations of Perl per test run. And it's even worse - it wasn't just a call to that Perl script, it was wrapped in a "cat | shasum | cut" pipeline, resulting in silent data loss when you accidentally passed multiple input files to the thing, 75,000 processes being spawned just to compute hashes and losing the file name of what was actually hashed. WTF. Also, we were using SHA256 to essentially verify that two directory trees are equal. For this purpose, relying on SHA1 should be absolutely fine - and that is, provided by a good native implementation, four times faster than `shasum`. It saves another 10 seconds of the overall run. With this change, the test also prints the result of a failed determinism check in an easier to read format "filename hash" instead of "hash filename" and on top of that, it also prints the filenames in the diff on macOS, which was missing formerly. Without this, it was basically impossible to debug failures of this test on macOS, as you couldn't see *which files were different*. You had *one* job, bazel_determinism_test. Before: ``` -- Test log: ----------------------------------------------------------- --- /private/var/tmp/_bazel_buildkite/30004132848cb6cbb0d8bc124cd9712b/bazel-sandbox/8820973750646175047/execroot/io_bazel/_tmp/e503f3f3df14b71e247bc3d7d9bf3608/sum1 2018-03-28 18:00:43.000000000 +0000 +++ /private/var/tmp/_bazel_buildkite/30004132848cb6cbb0d8bc124cd9712b/bazel-sandbox/8820973750646175047/execroot/io_bazel/_tmp/e503f3f3df14b71e247bc3d7d9bf3608/sum2 2018-03-28 18:10:34.000000000 +0000 @@ -10417,0 +10418 @@ +ecd53ba69a8d479d3fa4234e959f869cd10f7ebc68860d2b7915879f8b8b2c54 @@ -10605 +10605,0 @@ -f1954b59039b74d0a0ee3b2bced748604b95b8455a5bf80489296bd81878a5c8 ------------------------------------------------------------------------ ``` Now (I artificially introduced non-hermeticism to show how a failure would look like): ``` -- Test log: ----------------------------------------------------------- --- /private/var/tmp/_bazel_philwo/7a01905b4627ca044e5e3f5ad5b14d26/bazel-sandbox/5464595340038418595/execroot/io_bazel/_tmp/e503f3f3df14b71e247bc3d7d9bf3608/sum1 2018-03-30 17:12:39.000000000 +0000 +++ /private/var/tmp/_bazel_philwo/7a01905b4627ca044e5e3f5ad5b14d26/bazel-sandbox/5464595340038418595/execroot/io_bazel/_tmp/e503f3f3df14b71e247bc3d7d9bf3608/sum2 2018-03-30 17:17:27.000000000 +0000 @@ -903 +903 @@ -bazel-bin/src/bazel 31d811338ca364f0631560dd4d29406dd6a778ce +bazel-bin/src/bazel 8f009173894730b00a1d1d6349af7d10f4d21cf3 @@ -5656 +5656 @@ -bazel-bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar f5ec8c4415ad8ecdc0385affc68f2dd4dbf241ef +bazel-bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar 9899ae35cf431087a34a830bfdaf19d99616689c @@ -8343 +8343 @@ -bazel-bin/src/main/java/com/google/devtools/build/lib/worker/_javac/worker/libworker_classes/com/google/devtools/build/lib/worker/WorkerFactory.class 780baa17c19ef99ef0b9291db1791ed8e0f1b231 +bazel-bin/src/main/java/com/google/devtools/build/lib/worker/_javac/worker/libworker_classes/com/google/devtools/build/lib/worker/WorkerFactory.class d45c14f09e73e7fcdf01f96aa32646c87b704bc2 @@ -8359 +8359 @@ -bazel-bin/src/main/java/com/google/devtools/build/lib/worker/libworker.jar 60e3afbfec17da7e44c1f0f61cf2a446196717be +bazel-bin/src/main/java/com/google/devtools/build/lib/worker/libworker.jar 70f557e87d1b32b2e46c79554fe6bf3b89aeaf6e @@ -11343 +11343 @@ -bazel-genfiles/src/install_base_key 3fad754e4ea19bd1120df5bf16e1f39372e6b9fe +bazel-genfiles/src/install_base_key 7d7e8b62493912c5ec153032e104640e3980e6b3 @@ -11376 +11376 @@ -bazel-genfiles/src/package.zip 1ce3431b021ca338806162eca72ff84118001df5 +bazel-genfiles/src/package.zip 65f4801d91bbe10cba0d2d4d55c7cf319cd6722d ------------------------------------------------------------------------ test_determinism FAILED: Non-deterministic outputs found! . ``` ## Remove obsolete check for BAZEL_TEST_XTRACE That string does not appear anywhere in our repo, except for these two lines in the test, so there's no point in checking for it. ## Remove obsolete check for Java 7 That was about time. ## Performance improvements and usability fixes - There's no need to use mktemp to create a unique directory under TEST_TMPDIR, as every test suite has its own TEST_TMPDIR. - There's no need to remove stuff, as this will just degrade performance and make debugging harder. The surrounding Bazel or system will clean up later. - There's no need to copy bazel-bin/src/bazel to ./bazel1 before calling it, as you can just call the built bazel from its original location. - There's no need to run "bazel clean" before the second "bazel build" invocation - it's better to just use two separate output_bases. This is faster and also makes debugging easier, as you can compare the two output_bases in case of a test failure. - There's no need to call "diff" twice - we can just save the output immediately in the `if` block. Closes #4945. PiperOrigin-RevId: 191118833
Diffstat (limited to 'src/test/shell/testenv.sh')
-rwxr-xr-xsrc/test/shell/testenv.sh4
1 files changed, 2 insertions, 2 deletions
diff --git a/src/test/shell/testenv.sh b/src/test/shell/testenv.sh
index 870116a282..44fc1a07cf 100755
--- a/src/test/shell/testenv.sh
+++ b/src/test/shell/testenv.sh
@@ -230,11 +230,11 @@ exit 1;
case "${PLATFORM}" in
darwin|freebsd)
function sha256sum() {
- cat "$1" | shasum -a 256 | cut -f 1 -d " "
+ shasum -a 256 "$@"
}
;;
*)
- # Under linux sha256sum should exists
+ # Under Linux, sha256sum usually exists as a binary as part of coreutils.
;;
esac