From e25e4dc7e7589afa8aeea30d2ed62175a0e70ce7 Mon Sep 17 00:00:00 2001 From: Kevin Lubick Date: Tue, 7 Jun 2022 16:37:48 -0400 Subject: [PATCH] [bazel] Specify files required for the toolchain more precisely. This improves performance with sandboxing. One can have Bazel output performance data [1][2], which can be viewed via chrome://tracing or https://ui.perfetto.dev/. With Perfetto, the following SQL queries [3][4] can summarize the sandboxing metrics, as well as the actual compilation time. http://screen/5TxbeZTso4gNDfD Note that the dur column is in nanoseconds, so we convert to seconds. These numbers could further be divided by the number of processes (in my case 48) to get a scaled output. SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "sandbox.createFileSystem"; SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "sandbox.delete"; SELECT SUM(dur) / 1000000000.0 FROM slice WHERE name = "subprocess.run"; I benchmarked the local compilation of //:skia_public using --config=clang_linux (our custom Linux toolchain). I was sure to clear the Bazel cache before each run and not count the time to download/update a toolchain for the first time. The control measurements (without this CL) are: Wall Time = 272.2s sandbox.createFileSystem = 5466.9s subprocess.run = 2961.0s sandbox.delete = 4112.3s With this CL: Wall Time = 53.9s (5.05x faster) sandbox.createFileSystem = 403.4s subprocess.run = 1610.3s sandbox.delete = 348.8s The control measurement is a touch misleading. Due to it being so slow, I had recommended developers use a ramdisk when building on machines with sufficient RAM via the Bazel flag --sandbox_base=/dev/shm Here is the control measurement when using a RAM disk: Wall Time = 21.2s sandbox.createFileSystem = 58.9s subprocess.run = 705.1s sandbox.delete = 46.6s With this CL and a RAM disk: Wall Time = 19.2s (10% faster) sandbox.createFileSystem = 21.8s subprocess.run = 722.9s sandbox.delete = 16.2s For devs who cannot or are not using a RAM disk, this is a huge win. With a RAM disk, it's a modest improvement. On an RBE build, this had minimal impact. I did my best to bust the action cache with a fake define and the before and after times were about the same. This was inspired by [5] and [6]. [1] Add --profile=/tmp/profile.gz to any command [2] https://bazel.build/rules/performance#performance-profiling [3] https://perfetto.dev/docs/quickstart/trace-analysis#sample-queries [4] https://perfetto.dev/docs/analysis/sql-tables#slice [5] https://github.com/emscripten-core/emsdk/commit/93f21c9ef30bab52de24f9d4ea3f2f377cf6326a [6] https://github.com/emscripten-core/emsdk/commit/311acff345fd71dcfe5f350653cec466ee7e3fbc Change-Id: Iceb2606e86111159141a286d01fc002d09fe3fe4 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547822 Reviewed-by: Leandro Lovisolo --- toolchain/BUILD.bazel | 33 ++++++++---- toolchain/download_linux_amd64_toolchain.bzl | 57 +++++++++++++++++--- 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/toolchain/BUILD.bazel b/toolchain/BUILD.bazel index ce56bda801..9e99e0f650 100644 --- a/toolchain/BUILD.bazel +++ b/toolchain/BUILD.bazel @@ -7,8 +7,6 @@ licenses(["notice"]) exports_files_legacy() -package(default_visibility = ["//visibility:public"]) - # https://bazel.build/reference/be/c-cpp#cc_toolchain_suite # cc_toolchain_suite will fetch deps for toolchains it will not use, which # is why we split up the suites by OS. When attempting to fetch "@clang_linux_amd64//:all_files", @@ -37,13 +35,28 @@ cc_toolchain_suite( filegroup(name = "not_implemented") filegroup( - name = "all_linux_amd64_files", + name = "archive_linux_amd64_files", + srcs = [ + "linux_trampolines/ar_trampoline_linux.sh", + "@clang_linux_amd64//:archive_files", + ], +) + +filegroup( + name = "compile_linux_amd64_files", srcs = [ "linux_trampolines/IWYU_mapping.imp", - "linux_trampolines/ar_trampoline_linux.sh", "linux_trampolines/clang_trampoline_linux.sh", - "linux_trampolines/lld_trampoline_linux.sh", - "@clang_linux_amd64//:all_files", + "@clang_linux_amd64//:compile_files", + ], +) + +filegroup( + name = "link_linux_amd64_files", + srcs = [ + # Bazel assumes it is talking to Clang when linking. + "linux_trampolines/clang_trampoline_linux.sh", + "@clang_linux_amd64//:link_files", ], ) @@ -68,11 +81,11 @@ provide_mac_m1_toolchain_config( # https://bazel.build/reference/be/c-cpp#cc_toolchain cc_toolchain( name = "linux_amd64_host", - all_files = ":all_linux_amd64_files", - ar_files = ":all_linux_amd64_files", - compiler_files = ":all_linux_amd64_files", + all_files = ":not_implemented", + ar_files = ":archive_linux_amd64_files", + compiler_files = ":compile_linux_amd64_files", dwp_files = ":not_implemented", - linker_files = ":all_linux_amd64_files", + linker_files = ":link_linux_amd64_files", objcopy_files = ":not_implemented", strip_files = ":not_implemented", supports_param_files = False, diff --git a/toolchain/download_linux_amd64_toolchain.bzl b/toolchain/download_linux_amd64_toolchain.bzl index 06d6586ca5..0b8c924141 100644 --- a/toolchain/download_linux_amd64_toolchain.bzl +++ b/toolchain/download_linux_amd64_toolchain.bzl @@ -166,7 +166,7 @@ def _download_and_extract_deb(ctx, deb, sha256, prefix, output = ""): fail("Could not open deb.ar from " + deb) # https://bazel.build/rules/lib/repository_ctx#extract - extract_info = ctx.extract( + ctx.extract( archive = "tmp/data.tar.xz", output = output, stripPrefix = prefix, @@ -205,18 +205,59 @@ def _download_linux_amd64_toolchain_impl(ctx): ".", ) - # Create a BUILD.bazel file that makes all the files in this subfolder - # available for use in rules, i.e. in the toolchain declaration. + # Create a BUILD.bazel file that makes the files downloaded into the toolchain visible. + # We have separate groups for each task because doing less work (sandboxing fewer files + # or uploading less data to RBE) makes compiles go faster. We try to strike a balance + # between minimal specifications and not having to edit this file often with our use + # of globs. # https://bazel.build/rules/lib/repository_ctx#file ctx.file( "BUILD.bazel", content = """ filegroup( - name = "all_files", - srcs = glob([ - "**", - ]), - visibility = ["//visibility:public"] + name = "archive_files", + srcs = [ + "bin/llvm-ar", + ], + visibility = ["//visibility:public"], +) + +filegroup( + name = "compile_files", + srcs = [ + "bin/clang", + "usr/bin/include-what-you-use", + ] + glob( + include = [ + "include/c++/v1/**", + "usr/include/**", + "lib/clang/13.0.0/**", + "usr/include/x86_64-linux-gnu/**", + ], + allow_empty = False, + ), + visibility = ["//visibility:public"], +) + +filegroup( + name = "link_files", + srcs = [ + "bin/clang", + "bin/ld.lld", + "bin/lld", + "lib/libc++.a", + "lib/libc++abi.a", + "lib/libunwind.a", + "lib64/ld-linux-x86-64.so.2", + ] + glob( + include = [ + "lib/clang/13.0.0/lib/**", + "lib/x86_64-linux-gnu/**", + "usr/lib/x86_64-linux-gnu/**", + ], + allow_empty = False, + ), + visibility = ["//visibility:public"], ) """, executable = False,