Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix execution requirements for 'build without the bytes' #639

Merged
merged 1 commit into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
55 changes: 5 additions & 50 deletions lib/private/copy_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,63 +17,18 @@ def execution_requirements_for_copy(ctx):
# If the rule ctx doesn't expose the CopyOptions, the default is to run locally
return COPY_EXECUTION_REQUIREMENTS_LOCAL

# 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.
# When applied to execution_requirements of an action, these prevent the action from being sandboxed
# for improved performance.
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
# input.
"no-remote": "1",
"no-remote-cache": "1",
"no-remote-exec": "1",
"no-cache": "1",
# 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 execution.
"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
6 changes: 6 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,8 @@ copy_directory(
name = "tree_artifact",
src = "ds",
out = "dsc",
# RBE not happy with the symlinks in this test case
tags = ["no-remote"],
)

lib(
Expand Down Expand Up @@ -43,6 +45,8 @@ pkg(
":tree_artifact",
],
out = "pkg",
# RBE not happy with the symlinks in this test case
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 +63,8 @@ diff_test(
name = "test",
file1 = ":pkg",
file2 = ":expected_pkg",
# RBE not happy with the symlinks in this test case
tags = ["no-remote"],
)

bzl_library(
Expand Down