-
Notifications
You must be signed in to change notification settings - Fork 433
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
Persistent worker support #421
Changes from all commits
a88defb
ec046ba
7cdb169
12f8abc
55ee86a
8fa4e54
6b9ee79
715794b
b693c68
da0c8b8
801e2ae
77fe9c0
8a7b18a
e4d92e3
68c326c
068b6c9
3c2184b
1f79d09
a3e0ebb
a68c7e2
917bee9
81353d6
7eba058
96a0d34
7a487c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,6 +69,15 @@ Similarly, `rustfmt_version` may also be configured: | |
```python | ||
rust_repositories(rustfmt_version = "1.4.20") | ||
``` | ||
# Using Bazel Persistent Workers | ||
|
||
Iterating on Rust code may benefit from [incremental compilation](https://doc.rust-lang.org/edition-guide/rust-2018/the-compiler/incremental-compilation-for-faster-compiles.html). This is supported by using a [Bazel Persistent Worker](https://docs.bazel.build/versions/master/persistent-workers.html). While Bazel can determine what needs to be rebuilt at a crate level, the compiler can speed up building a single crate by sharing information across runs. It does this by storing intermediate information in a directory across invocations. This is enabled by default in Cargo. Persistent workers bring this feature to Bazel. | ||
|
||
The Rust rules have preliminary support for workers. Pass `use_worker = True` to enable this when available. | ||
|
||
```python | ||
rust_repositories(use_worker = True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see why this shouldn't be the default, was there a reason to be conservative about this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's pulling in and running a binary hosted on a third-party repo, so off by default is probably the best option. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, what @dae said. Plus given that only 2(?) people have actually used it, it seemed very alpha. |
||
``` | ||
|
||
## External Dependencies | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
load("@bazel_skylib//:bzl_library.bzl", "bzl_library") | ||
load("//worker:toolchain.bzl", "worker_toolchain") | ||
|
||
package(default_visibility = ["//visibility:public"]) | ||
|
||
exports_files([ | ||
"repositories.bzl", | ||
]) | ||
|
||
bzl_library( | ||
name = "rules", | ||
srcs = glob(["**/*.bzl"]), | ||
) | ||
|
||
toolchain_type(name = "toolchain_type") | ||
|
||
worker_toolchain(name = "worker_dummy") | ||
|
||
toolchain( | ||
name = "dummy", | ||
toolchain = ":worker_dummy", | ||
toolchain_type = ":toolchain_type", | ||
) | ||
|
||
# These toolchains are only registered if workers are enabled in rust_repositories(). | ||
|
||
worker_toolchain( | ||
name = "worker_linux_x86_64", | ||
worker_binary = "@rust_worker_linux_x86_64//file", | ||
) | ||
|
||
toolchain( | ||
name = "linux_x86_64", | ||
exec_compatible_with = [ | ||
"@platforms//os:linux", | ||
"@platforms//cpu:x86_64", | ||
], | ||
toolchain = ":worker_linux_x86_64", | ||
toolchain_type = ":toolchain_type", | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
|
||
# Rust Persistent Worker | ||
|
||
The Rust Persistent Worker is itself implemented in Rust. It is built via Cargo and distributed as binaries. Source and release binaries are maintained at the [rustc-worker project](https://github.com/nikhilm/rustc-worker). Contributions should be submitted there and then the version of the binaries updated in `worker/repositories.bzl`. | ||
|
||
## Why isn't this built via Bazel? | ||
|
||
Bootstrapping the worker using these same rules (e.g. `rust_binary`) may be possible, but is not supported right now. There are a couple of challenges: | ||
1. Since the worker has dependencies on various crates, it uses cargo-raze, which generates relevant rules. This means "don't use the worker to build this target" is transitive and such information needs to be propagated down the tree in a way that works with restrictions in Bazel. Initial experiments repeatedly encountered failures due to Bazel treating the rule dependency on the worker executable target as a cycle, even when building in non-worker mode. This may be user error or a restriction in Bazel. Until that is addressed, the easiest way to fix this is to change cargo-raze to customize what rules are used, and provide `rust_no_worker_binary` and `rust_no_worker_library` rules that do not have this cycle. | ||
2. Figuring out a good strategy for dependencies. Since Bazel doesn't really have transitive dependencies, attempts to build this worker from source necessarily require users of these rules to register all the external repositories for the worker in their `WORKSPACE`. This could cause collisions with other dependencies. In addition, if the `_no_worker_` approach above is adopted, users will lose the benefits of workers for those dependencies (like `protobuf`) shared between the worker and their code. There is no satisfactory solution for this right now. | ||
|
||
## How about rewriting the worker in C++? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the worker benefit from being written in a non-gc'd language? Curious about whether python is a reasonable choice, given other utilities in rules_rust have been written in python. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, there is nothing preventing writing this in Python for someone with such an interest. I did not find any references to Python rules in this repo. I suppose as a bootstrap language (i.e. one that already has Bazel rules for it), that is a better alternative than C++. |
||
|
||
That is certainly an option! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
# buildifier: disable=module-docstring | ||
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_file") | ||
|
||
def rust_worker_repositories(): | ||
"""Registers rustc-worker binary archives.""" | ||
http_file( | ||
name = "rust_worker_linux_x86_64", | ||
executable = True, | ||
sha256 = "c7ee178d658a9ff9c9b10f7acce48af57227170d454741072aff5fabf923f8fb", | ||
urls = ["https://github.com/nikhilm/rustc-worker/releases/download/v0.2.0/rustc-worker-0.2.0-linux-x86_64"], | ||
) | ||
|
||
# buildifier: disable=unnamed-macro | ||
def rust_worker_toolchains(): | ||
"""Registers worker toolchains for supported platforms.""" | ||
native.register_toolchains( | ||
"@io_bazel_rules_rust//worker:linux_x86_64", | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# buildifier: disable=module-docstring | ||
def _worker_toolchain_impl(ctx): | ||
toolchain_info = platform_common.ToolchainInfo( | ||
worker_binary = ctx.executable.worker_binary, | ||
) | ||
return [toolchain_info] | ||
|
||
worker_toolchain = rule( | ||
implementation = _worker_toolchain_impl, | ||
attrs = { | ||
"worker_binary": attr.label(allow_single_file = True, executable = True, cfg = "exec"), | ||
}, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is linux only, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I posted a Mac version above - but each platform would need its own binary at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rustc-worker also has a mac build available now. I can update this PR with an entry for that.