From 83cde24f7c7af23bbf0e160144ae23d933440749 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Fri, 6 Oct 2023 14:19:42 -0700 Subject: [PATCH 1/2] Revert "refactor: reduce execution requirements for copy" This reverts commit 40c2ddc71aab51e82a64d4893af874ef3dd10173. --- .github/workflows/ci.bazelrc | 1 + lib/private/copy_common.bzl | 56 ++++++++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.bazelrc b/.github/workflows/ci.bazelrc index 971e7b2b4..e6e0c7411 100644 --- a/.github/workflows/ci.bazelrc +++ b/.github/workflows/ci.bazelrc @@ -9,6 +9,7 @@ build:local --disk_cache=~/.cache/bazel # Remote build execution build:rbe --extra_execution_platforms=@aspect_bazel_lib//platforms:x86_64_linux_remote +build:rbe --genrule_strategy=remote build:rbe --host_platform=@aspect_bazel_lib//platforms:x86_64_linux_remote build:rbe --jobs=32 diff --git a/lib/private/copy_common.bzl b/lib/private/copy_common.bzl index 157a1c24b..aedb04ac6 100644 --- a/lib/private/copy_common.bzl +++ b/lib/private/copy_common.bzl @@ -1,11 +1,61 @@ "Helpers for copy rules" -# Hints for Bazel spawn strategy, copied from -# https://github.com/bazelbuild/bazel-skylib/blob/0171c69e5cc691e2d0cd9f3f3e4c3bf112370ca2/rules/private/copy_common.bzl -# See extensive comments there for reasoning on this execution-requirements selection. +# Hints for Bazel spawn strategy COPY_EXECUTION_REQUIREMENTS = { + # ----------------+----------------------------------------------------------------------------- + # no-remote | Prevents the action or test from being executed remotely or cached remotely. + # | This is equivalent to using both `no-remote-cache` and `no-remote-exec`. + # ----------------+----------------------------------------------------------------------------- + # no-remote-cache | Results in the action or test never being cached remotely (but it may + # | be cached locally; it may also be executed remotely). Note: for the purposes + # | of this tag, the disk-cache is considered a local cache, whereas the http + # | and gRPC caches are considered remote. If a combined cache is specified + # | (i.e. a cache with local and remote components), it's treated as a remote + # | cache and disabled entirely unless --incompatible_remote_results_ignore_disk + # | is set in which case the local components will be used. + # ----------------+----------------------------------------------------------------------------- + # no-remote-exec | Results in the action or test never being executed remotely (but it may be + # | cached remotely). + # ----------------+----------------------------------------------------------------------------- + # no-cache | Results in the action or test never being cached (remotely or locally) + # ----------------+----------------------------------------------------------------------------- + # no-sandbox | Results in the action or test never being sandboxed; it can still be cached + # | or run remotely - use no-cache or no-remote to prevent either or both of + # | those. + # ----------------+----------------------------------------------------------------------------- + # local | Precludes the action or test from being remotely cached, remotely executed, + # | or run inside the sandbox. For genrules and tests, marking the rule with the + # | local = True attribute has the same effect. + # ----------------+----------------------------------------------------------------------------- + # See https://bazel.google.cn/reference/be/common-definitions?hl=en&authuser=0#common-attributes + # + # Copying file & directories is entirely IO-bound and there is no point doing this work + # remotely. + # + # Also, remote-execution does not allow source directory inputs, see + # https://github.com/bazelbuild/bazel/commit/c64421bc35214f0414e4f4226cc953e8c55fa0d2 So we must + # not attempt to execute remotely in that case. + # + # There is also no point pulling the output file or directory from the remote cache since the + # bytes to copy are already available locally. Conversely, no point in writing to the cache if + # no one has any reason to check it for this action. + # + # Read and writing to disk cache is disabled as well primarily to reduce disk usage on the local + # machine. A disk cache hit of a directory copy could be slghtly faster than a copy since the + # disk cache stores the directory artifact as a single entry, but the slight performance bump + # comes at the cost of heavy disk cache usage, which is an unmanaged directory that grow beyond + # the bounds of the physical disk. + # TODO: run benchmarks to measure the impact on copy_directory + # + # Sandboxing for this action is wasteful as well since there is a 1:1 mapping of input + # file/directory to output file/directory and no room for non-hermetic inputs to sneak in to the + # input. "no-remote": "1", + "no-remote-cache": "1", + "no-remote-exec": "1", "no-cache": "1", + "no-sandbox": "1", + "local": "1", } def progress_path(f): From 8cb022f6ffbadbe69bcb6df95e75deabc6e3c985 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Fri, 6 Oct 2023 14:34:23 -0700 Subject: [PATCH 2/2] Update ci.bazelrc --- .github/workflows/ci.bazelrc | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.bazelrc b/.github/workflows/ci.bazelrc index e6e0c7411..971e7b2b4 100644 --- a/.github/workflows/ci.bazelrc +++ b/.github/workflows/ci.bazelrc @@ -9,7 +9,6 @@ build:local --disk_cache=~/.cache/bazel # Remote build execution build:rbe --extra_execution_platforms=@aspect_bazel_lib//platforms:x86_64_linux_remote -build:rbe --genrule_strategy=remote build:rbe --host_platform=@aspect_bazel_lib//platforms:x86_64_linux_remote build:rbe --jobs=32