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

Make COPY_EXECUTION_REQUIREMENTS optional #533

Open
sluongng opened this issue Aug 6, 2024 · 2 comments
Open

Make COPY_EXECUTION_REQUIREMENTS optional #533

sluongng opened this issue Aug 6, 2024 · 2 comments

Comments

@sluongng
Copy link

sluongng commented Aug 6, 2024

Currently, we have

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-cache | Results in the action or test never being cached (remotely or locally)
# ----------------+-----------------------------------------------------------------------------
# See https://bazel.build/reference/be/common-definitions#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.
"no-remote": "1",
"no-cache": "1",
}
which disable remote cache, remote execution, and cache for the Copy* actions.

This is a sane default for a local build, but for a fully remote build with Build without the Bytes turned on, this requirement causes Bazel to fetch the inputs locally, run the action, and upload the outputs. As a result, the build is dramatically slowed down compared to running these actions remotely and letting the RBE system handle the optimization.

I think it would make more sense if we create a separate execution group for these copy actions. The downstream rules/users could decide the default value of these execution properties for the copy group, or to override them.

https://bazel.build/extending/exec-groups

@tetromino
Copy link
Collaborator

CC-ing @katre for platform expertise

@katre
Copy link
Member

katre commented Aug 12, 2024

I think this makes sense, where are these used from? Are they parts of other rules? Can we implement this more cleanly with subrules?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants