Skip to content

Commit

Permalink
incremental compilation without a worker
Browse files Browse the repository at this point in the history
This is considerably less invasive than bazelbuild#667, bazelbuild#517 and bazelbuild#421 - there
is no extra code to bootstrap, and no toolchains have to be plumbed
into the rules.

We had been using a worker to get around sandboxing restrictions, but
as @hlopko pointed out on bazelbuild#667, that turns out not to be necessary -
even normal rules have access to /tmp. Unfortunately it seems that rustc
does not expect the source files to change location, and it will
consistently crash when used in a sandboxed context. So the approach
this PR takes is to disable sandboxing on targets that are being compiled
incrementally. This fixes the compiler crashes, and as a bonus, means
we're not limited to saving the cache in /tmp.

This PR adds a --@rules_rust//:experimental_incremental_base flag to
specify the path where incremental build products are stored - if not
provided, the rules will function as they normally do.

The default behaviour will incrementally compile crates in the local
workspace, like cargo does. The behaviour can be adjusted with another
flag, which is covered in docs/index.md.
  • Loading branch information
dae authored and vmax committed Aug 25, 2021
1 parent d2bf64f commit 9577c14
Show file tree
Hide file tree
Showing 6 changed files with 193 additions and 5 deletions.
20 changes: 19 additions & 1 deletion BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load("//rust:rust.bzl", "error_format")
load(
"//rust:defs.bzl",
"error_format",
"experimental_incremental_base",
"experimental_incremental_prefixes",
)

exports_files(["LICENSE"])

Expand Down Expand Up @@ -37,3 +42,16 @@ alias(
actual = "//tools/rustfmt",
visibility = ["//visibility:public"],
)

# Optional incremental compilation - see docs/index.md
experimental_incremental_base(
name = "experimental_incremental_base",
build_setting_default = "",
visibility = ["//visibility:public"],
)

experimental_incremental_prefixes(
name = "experimental_incremental_prefixes",
build_setting_default = "//,-//vendored//",
visibility = ["//visibility:public"],
)
1 change: 1 addition & 0 deletions docs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package(default_visibility = ["//visibility:private"])
bzl_library(
name = "docs_deps",
srcs = [
"@bazel_skylib//rules:common_settings",
"@bazel_tools//tools:bzl_srcs",
"@build_bazel_rules_nodejs//internal/providers:bzl",
"@com_google_protobuf//:bzl_srcs",
Expand Down
26 changes: 26 additions & 0 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,29 @@ rust_repositories(rustfmt_version = "1.53.0")

Currently, the most common approach to managing external dependencies is using
[cargo-raze](https://github.com/google/cargo-raze) to generate `BUILD` files for Cargo crates.


## Incremental Compilation

There is an experimental flag that enables incremental compilation, which can
considerably speed up rebuilds during development.

Targets built with incremental compilation enabled will have sandboxing
disabled, so enabling this option is trading off some of Bazel's hermeticity in
the name of performance. This option is not recommended for CI or release
builds.

To enable incremental compilation, add the following argument to your bazel
build command, adjusting the directory path to one that suits:

--@rules_rust//:experimental_incremental_base=/home/user/cache/bazel_incremental

A separate flag allows you to control which crates are incrementally compiled.
The default is:

--@rules_rust//:experimental_incremental_prefixes=//,-//vendored

This will incrementally compile any crates in the local workspace, but exclude
other repositories like `@raze__*`, and anything in the //vendored package. This
behaviour is similar to cargo, which only incrementally compiles the local
workspace.
8 changes: 8 additions & 0 deletions rust/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ load(
load(
"//rust/private:rustc.bzl",
_error_format = "error_format",
_incremental_base = "incremental_base",
_incremental_prefixes = "incremental_prefixes",
)
load(
"//rust/private:rustdoc.bzl",
Expand Down Expand Up @@ -97,6 +99,12 @@ rust_clippy = _rust_clippy
error_format = _error_format
# See @rules_rust//rust/private:rustc.bzl for a complete description.

experimental_incremental_base = _incremental_base
# See @rules_rust//rust/private:rustc.bzl for a complete description.

experimental_incremental_prefixes = _incremental_prefixes
# See @rules_rust//rust/private:rustc.bzl for a complete description.

rust_common = _rust_common
# See @rules_rust//rust/private:common.bzl for a complete description.

Expand Down
26 changes: 25 additions & 1 deletion rust/private/rust.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
# limitations under the License.

# buildifier: disable=module-docstring
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load("//rust/private:common.bzl", "rust_common")
load("//rust/private:rustc.bzl", "rustc_compile_action")
load(
"//rust/private:utils.bzl",
"crate_name_from_attr",
Expand All @@ -23,6 +23,7 @@ load(
"expand_dict_value_locations",
"find_toolchain",
)
load("//rust/private:rustc.bzl", "IncrementalInfo", "IncrementalPrefixInfo", "rustc_compile_action")

# TODO(marco): Separate each rule into its own file.

Expand Down Expand Up @@ -103,6 +104,24 @@ def _determine_lib_name(name, crate_type, toolchain, lib_hash = ""):
extension = extension,
)

def incremental_info(attr):
"""Extract incremental compilation info from common rule attributes.
Expects to find the following attributes:
_incremental_base
_incremental_prefixes
Returns:
IncrementalInfo: incremental compilation configuration
"""
prefixes = attr._incremental_prefixes[IncrementalPrefixInfo]
base = attr._incremental_base[BuildSettingInfo].value
return IncrementalInfo(
prefixes = prefixes,
base = base,
)

def get_edition(attr, toolchain):
"""Returns the Rust edition from either the current rule's attirbutes or the current `rust_toolchain`
Expand Down Expand Up @@ -269,6 +288,7 @@ def _rust_library_common(ctx, crate_type):
compile_data = depset(ctx.files.compile_data),
),
output_hash = output_hash,
incremental_info = incremental_info(ctx.attr),
)

def _rust_binary_impl(ctx):
Expand Down Expand Up @@ -304,6 +324,7 @@ def _rust_binary_impl(ctx):
is_test = False,
compile_data = depset(ctx.files.compile_data),
),
incremental_info = incremental_info(ctx.attr),
)

def _create_test_launcher(ctx, toolchain, output, providers):
Expand Down Expand Up @@ -453,6 +474,7 @@ def _rust_test_common(ctx, toolchain, output):
toolchain = toolchain,
crate_info = crate_info,
rust_flags = ["--test"] if ctx.attr.use_libtest_harness else ["--cfg", "test"],
incremental_info = incremental_info(ctx.attr),
)

return _create_test_launcher(ctx, toolchain, output, providers)
Expand Down Expand Up @@ -663,6 +685,8 @@ _common_attrs = {
default = "@bazel_tools//tools/cpp:current_cc_toolchain",
),
"_error_format": attr.label(default = "//:error_format"),
"_incremental_base": attr.label(default = "//:experimental_incremental_base"),
"_incremental_prefixes": attr.label(default = "//:experimental_incremental_prefixes"),
"_process_wrapper": attr.label(
default = Label("//util/process_wrapper"),
executable = True,
Expand Down
117 changes: 114 additions & 3 deletions rust/private/rustc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

# buildifier: disable=module-docstring
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
load(
"@bazel_tools//tools/build_defs/cc:action_names.bzl",
"CPP_LINK_EXECUTABLE_ACTION_NAME",
Expand Down Expand Up @@ -50,6 +51,22 @@ AliasableDepInfo = provider(
},
)

IncrementalInfo = provider(
doc = "Data relating to incremental compilation.",
fields = {
"base": "string: base folder to store incremental build products",
"prefixes": "IncrementalPrefixInfo: prefixes to include and exclude",
},
)

IncrementalPrefixInfo = provider(
doc = "Prefixes to include and exclude in incremental compilation.",
fields = {
"exclude": "List[string]: prefixes that will exclude a label if matched",
"include": "List[string]: prefixes that will include a label if matched",
},
)

_error_format_values = ["human", "json", "short"]

ErrorFormatInfo = provider(
Expand Down Expand Up @@ -537,7 +554,8 @@ def rustc_compile_action(
crate_info,
output_hash = None,
rust_flags = [],
environ = {}):
environ = {},
incremental_info = None):
"""Create and run a rustc compile action based on the current rule's attributes
Args:
Expand All @@ -548,6 +566,7 @@ def rustc_compile_action(
output_hash (str, optional): The hashed path of the crate root. Defaults to None.
rust_flags (list, optional): Additional flags to pass to rustc. Defaults to [].
environ (dict, optional): A set of makefile expandable environment variables for the action
incremental_info (str, optional): path to store incremental build products in.
Returns:
list: A list of the following providers:
Expand Down Expand Up @@ -597,14 +616,34 @@ def rustc_compile_action(
else:
formatted_version = ""

if (incremental_info and
incremental_info.base and
_want_incremental_compile(ctx.label, incremental_info.prefixes)):
incremental_dir = "{}/{}_{}".format(
incremental_info.base,
ctx.var["COMPILATION_MODE"],
toolchain.target_triple,
)
args.all.extend(["--codegen", "incremental=" + incremental_dir])

# with sandboxing enabled, subsequent rustc invocations will crash,
# as it doesn't expect the source files to have moved
execution_requirements = {"no-sandbox": "1"}
mnemonic = "RustIncr"
else:
execution_requirements = {}
mnemonic = "Rustc"

ctx.actions.run(
executable = ctx.executable._process_wrapper,
inputs = compile_inputs,
outputs = [crate_info.output],
env = env,
arguments = args.all,
mnemonic = "Rustc",
progress_message = "Compiling Rust {} {}{} ({} files)".format(
mnemonic = mnemonic,
execution_requirements = execution_requirements,
progress_message = "Compiling {} {} {}{} ({} files)".format(
mnemonic,
crate_info.type,
ctx.label.name,
formatted_version,
Expand Down Expand Up @@ -1022,3 +1061,75 @@ error_format = rule(
implementation = _error_format_impl,
build_setting = config.string(flag = True),
)

def _incremental_base_impl(ctx):
"""Implementation for the incremental_base() rule
Args:
ctx (ctx): The rule's context object
Returns:
BuildSettingInfo: an object with a `value` attribute containing the string.
"""
value = ctx.build_setting_value
return BuildSettingInfo(value = value)

incremental_base = rule(
build_setting = config.string(flag = True),
implementation = _incremental_base_impl,
doc = "Declares a command line argument that accepts an arbitrary string.",
)

def _incremental_prefixes_impl(ctx):
"""Implementation for the incremental_prefixes_flag() rule
Splits the provided string on commas, and then partitions prefixes starting
with a hypen into the exclude list, returning a provider with the include
and exclude list. The hypens are stripped from the entries in the exclude list.
Args:
ctx (ctx): The rule's context object
Returns:
(IncrementalPrefixInfo): a list of prefixes to include and exclude
"""
values = ctx.build_setting_value.split(",")
include = []
exclude = []
for value in ctx.build_setting_value.split(","):
if not value:
continue
elif value.startswith("-"):
exclude.append(value[1:])
else:
include.append(value)
return IncrementalPrefixInfo(include = include, exclude = exclude)

incremental_prefixes = rule(
build_setting = config.string(flag = True),
implementation = _incremental_prefixes_impl,
doc = """Declares a command line argument for incremental prefixes.
See _incremental_prefixes_impl() for the details.
""",
)

def _want_incremental_compile(label, prefixes):
"""True if the provided prefixes indicate the target should be incrementally compiled.
Args:
label (Label): the label for a given target
prefixes (IncrementalPrefixInfo): prefixes to include and exclude
Returns:
bool
"""
label = str(label)
for prefix in prefixes.exclude:
if label.startswith(prefix):
return False
for prefix in prefixes.include:
if label.startswith(prefix):
return True

return False

0 comments on commit 9577c14

Please sign in to comment.