Skip to content

Commit

Permalink
fix: fix execution requirements for 'build without the bytes'
Browse files Browse the repository at this point in the history
  • Loading branch information
gregmagolan committed Oct 30, 2023
1 parent 1365cfd commit 2d9b19f
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 65 deletions.
9 changes: 0 additions & 9 deletions .aspect/bazelrc/performance.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,3 @@ build --experimental_reuse_sandbox_directories
# author.
# Docs: https://bazel.build/reference/command-line-reference#flag--legacy_external_runfiles
build --nolegacy_external_runfiles

# Some actions are always IO-intensive but require little compute. It's wasteful to put the output
# in the remote cache, it just saturates the network and fills the cache storage causing earlier
# evictions. It's also not worth sending them for remote execution.
# For actions like PackageTar it's usually faster to just re-run the work locally every time.
# You'll have to look at an execution log to figure out what other action mnemonics you care about.
# In some cases you may need to patch rulesets to add a mnemonic to actions that don't have one.
# https://bazel.build/reference/command-line-reference#flag--modify_execution_info
build --modify_execution_info=PackageTar=+no-remote
50 changes: 3 additions & 47 deletions lib/private/copy_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,60 +20,16 @@ def execution_requirements_for_copy(ctx):
# When applied to execution_requirements of an action, these prevent the action from being
# sandboxed or remotely cached, for performance of builds that don't rely on RBE and build-without-bytes.
COPY_EXECUTION_REQUIREMENTS_LOCAL = {
# ----------------+-----------------------------------------------------------------------------
# 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.
# | or run remotely.
# ----------------+-----------------------------------------------------------------------------
# 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
# Sandboxing for this action is wasteful since there is a 1:1 mapping of input
# file/directory to output file/directory so little 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):
Expand Down
9 changes: 0 additions & 9 deletions lib/tests/bazelrc_presets/all/performance.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,3 @@ build --experimental_reuse_sandbox_directories
# author.
# Docs: https://bazel.build/reference/command-line-reference#flag--legacy_external_runfiles
build --nolegacy_external_runfiles

# Some actions are always IO-intensive but require little compute. It's wasteful to put the output
# in the remote cache, it just saturates the network and fills the cache storage causing earlier
# evictions. It's also not worth sending them for remote execution.
# For actions like PackageTar it's usually faster to just re-run the work locally every time.
# You'll have to look at an execution log to figure out what other action mnemonics you care about.
# In some cases you may need to patch rulesets to add a mnemonic to actions that don't have one.
# https://bazel.build/reference/command-line-reference#flag--modify_execution_info
build --modify_execution_info=PackageTar=+no-remote
3 changes: 3 additions & 0 deletions lib/tests/copy_to_directory_bin_action/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ copy_directory(
name = "tree_artifact",
src = "ds",
out = "dsc",
tags = ["no-remote"],
)

lib(
Expand Down Expand Up @@ -43,6 +44,7 @@ pkg(
":tree_artifact",
],
out = "pkg",
tags = ["no-remote"],
target_compatible_with = select({
# D:/a/bazel-lib/bazel-lib/lib/tests/copy_to_directory_bin_action/BUILD.bazel:36:4:
# declared output 'lib/tests/copy_to_directory_bin_action/pkg_symlink_0' is not a symlink
Expand All @@ -59,6 +61,7 @@ diff_test(
name = "test",
file1 = ":pkg",
file2 = ":expected_pkg",
tags = ["no-remote"],
)

bzl_library(
Expand Down
2 changes: 2 additions & 0 deletions lib/tests/copy_to_directory_bin_action/pkg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ def _pkg_impl(ctx):
copy_to_directory_bin = ctx.executable._tool,
hardlink = "auto",
verbose = True,
# RBE not happy with the symlinks in this test
override_execution_requirements = {"no-remote": "1"},
)

return [
Expand Down

0 comments on commit 2d9b19f

Please sign in to comment.