Implementing Rustc worker for rules_rust

[Marcel Hlopko]

Hi all,

there's important (and interesting :) discussion going on in https://github.com/bazelbuild/rules_rust/pull/684 about supporting workers. I'd like the discussion to continue here where it should have better visibility than comments in a PR. My last message:
--
I've spent some time today to learn more about Bazel workers and I think I came full circle :) I'm also sorry for not knowing this already, I could probably save you all some time. If you want to follow along, I've read:

* https://docs.bazel.build/versions/master/persistent-workers.html
* https://docs.bazel.build/versions/master/creating-workers.html (quite useful!)
* https://blog.bazel.build/2019/02/01/dynamic-spawn-scheduler.html

My summary:

The approach in this PR is actually not using Bazel's worker support, but rather reimplementing it for the Rustc specifics. The defining point of Bazel workers is that workers are long running programs (servers actually) that receive requests in proto or json and return responses. If rules support workers, whether the worker is used or not is controlled by Bazel's execution strategy. For example by passing --strategy=Rustc=worker,local we tell Bazel that (some, read on) Rustc actions can use worker. When rules register actions, they don't know if the worker will be used or not. Actions can specify their execution requirements and there they can say if they support workers or not. If not, worker is not used. If yes, worker is maybe used depending on execution strategies.

The main reason why it matters (to me at least) is the dynamic strategy (https://blog.bazel.build/2019/02/01/dynamic-spawn-scheduler.html). With that you can tell Bazel to use both the worker and the remote execution service, and whichever is faster wins. So in the case of a clean build, worker is probably going to be slower because of limited parallelism of the host machine, but for the incremental build it can be faster because maybe the remote execution workers are not as performant for single core workloads as the host machine. In our experience dynamic strategy can speed up incremental builds enough to be worth the added complexity. Dynamic strategy is not usable with the approach implemented in this PR (unfortunately).

I think it's desirable to implement Rustc worker support using the Bazel conventions. Rustc is a bit unique as a worker since it doesn't need to stay running, it stores all its data into a directory and the next invocation can read it. That is why it was possible to write this PR. But I can see a future improvement to Rustc to support server mode to keep the data in the memory and not spend action execution time by serializing and deserializing. And we would be able to benefit from the dynamic strategy.

What it all means is that we will need a rustc wrapper. It will need to parse and write JSON. It can be multithreaded (but workers don't have to be in general, it is only relevant for https://docs.bazel.build/versions/2.0.0/multiplex-worker.html, and we need to investigate if multiplex workers have benefits for rustc workloads).

I propose the following (and I'm prepared to invest my time into that unless there is somebody driven to work on it):

* solve the bootstrap problem
* rewrite process_wrapper in rust
* extend the process wrapper to support the worker mode
* modify the rules to always support worker mode

What do you all think?

--

Marcel Hlopko | Software Engineer | hlo...@google.com | 

Google Germany GmbH | Erika-Mann-Str. 33  | 80636 München | Germany | Geschäftsführer: Geschäftsführer: Paul Manicle, Halimah DeLaine Prado | Registergericht und -nummer: Hamburg, HRB 86891

[Daniel Wagner-Hall]

This sounds good to me. One thing I'd like to suggest (which I suspect is part of your "solve the bootstrap problem" bullet point) is that the cargo build script runner is somewhat restrictive to work on because of its zero-dependencies requirement, and it would be great not to need to stick to that for the process_wrapper - it'd be interesting if you wanted to try out `crate_universe` in anger here, and I'm happy to help out with that if needed!

[Nikhil Marathe]

Thanks for starting this conversation Marcel!

It seems that we do need a "proper" worker to take advantage of these strategies (which I didn't even know about!).

I think there are some orthogonal points you are raising and I just want to point them out:

1. Let's add worker support for rustc.

2. Let's make it bootstrapped.

3. Let's fold the functionality into process-wrapper, instead of a separate binary.

4. Let's make it use JSON.

5. Let's make it multi-threaded.

It might be more effective to pursue them one at a time :)

Point 1 is already implemented in https://github.com/bazelbuild/rules_rust/pull/667. This should be enough to validate your hunch about strategies. It doesn't rely on bootstrap and the primary problem is solving https://github.com/bazelbuild/rules_rust/pull/667#issuecomment-811852618 related to compiler crashes due to something that implementation is doing incorrectly. I suggest we use that as the place to start from.

Point 2 - There is already a rust implementation of the worker in https://github.com/bazelbuild/rules_rust/pull/421 and you could definitely use it as the thing to bootstrap from. It relied on pre-built binaries because I did not want to deal with bootstrapping. However, I don't see the value in spending effort bootstrapping it apart from "now it is in Rust" :) However, this is open source after all and we are all contributing because we enjoy it, so do what you want :) 

Point 3 - Yes this is definitely something that should be done in the long term. I have avoided doing it in both #421 and #667 because having a separate process was far easier to prototype things and not break the non-worker part. However, 2 process spawns are certainly less efficient than 1, particularly on Windows where process execution is considerably slower [1]. It should be pretty easy to move everything into one binary and support both modes of execution (worker binaries should support execution as non-workers).

Point 4 - I don't have strong opinions on this. protobufs are easier in C++ because the rules_rust toolchain already has it set up, whereas json will require pulling in some other lib. Plus they are strongly typed, but that doesn't matter so much for the current protocol. In a pure-Rust worker, you would need to pull in a library either way and solve the bootstrap problem.

Point 5 - Re: Windows processes, this may actually make things even faster on Windows iff Bazel is smart enough to prefer threads over processes when a worker supports it.

So #1 is enough to get you going on your dynamic strategy test. #3 and #5 are potential performance improvements. #4 is entirely a matter of preference. #2 seems mostly like a nice-to-have to me.

I don't think I'll be investing more dedicated time into rules_rust as I'm starting a new job soon that will not involve Rust. However I am really interested in pushing Bazel wherever possible, even though Cargo is entrenched in the Rust ecosystem, and would be delighted to see things that make it less of a compromise (i.e. missing incremental builds). Please feel free to drive this if you have the resources! Thanks again.

[1]: https://gregoryszorc.com/blog/2021/04/06/surprisingly-slow/ ("New Process overhead on Windows")

[Marcel Hlopko]

Oh time flies.

Daniel: I hear you. Writing the process wrapper without any crate will certainly be restrictive. We'll talk :)

Nikhil: First of all, thanks for a thorough reply!

Yes, my intention was to pursue those things one at a time. Your reinterpretation though makes it obvious how irrational I am about writing stuff in Rust. I'm almost persuaded to reorder the plan to do the Rust rewrite after 3) or 4).

Re 4) using protobuf has a price, currently one has to build protoc before generating C++ protos. This is going to regress the duration of the first build of the day when working on rules rust. I personally don't mind since I typically work on a decent workstation, but people working on not so performant laptops may be unhappy.

5) agreed.

Thanks for the update on your capacity. If you want, I can at least CC you on the work.

Others: any feedback you wanted to give?

--
You received this message because you are subscribed to the Google Groups "rules_rust" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rules_rust+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rules_rust/245c8212-46c9-404a-8058-5200e91b7243n%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Andre Brisco]

> Point 2 - There is already a rust implementation of the worker in https://github.com/bazelbuild/rules_rust/pull/421 and you could definitely use it as the thing to bootstrap from. It relied on pre-built binaries because I did not want to deal with bootstrapping. However, I don't see the value in spending effort bootstrapping it apart from "now it is in Rust" :) However, this is open source after all and we are all contributing because we enjoy it, so do what you want :)

I think one of the biggest values of Rust of C++ is the guarantees it can make that C++ can't. I'm happy to deal with bootstrapping issues if it means we can write something in Rust that would have a higher chance of being bug free :D

I don't think the desire of wanting everything to be in Rust is irrational (well, ok, there are some lines to be drawn, but this isn't one of them IMO).