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.

Sign up for GitHub

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

Jump to bottom

Persistent worker in C++ #667

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Persistent worker in C++ #667

wants to merge 18 commits into from
+410 −2

Commits on Mar 31, 2021

  1. new attempt at rust persistent worker: just the build setting for now 

    going to try this in C++ and have it all work nicely.
This commit just sets up a build setting, so that users can customize
whether to use the worker by just setting the flag on the command line
or in a local bazelrc file. This allows easily switching between
worker and non-worker.

Use as `bazel build <rust target> --@rules_rust//rust:use-worker`.
Obviously this will fail right now.
    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    1ab68b2 View commit details
    Browse the repository at this point in the history

  2. Now capable of parsing the WorkRequest

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    d0cd026 View commit details
    Browse the repository at this point in the history

  3. write fake worker responses. 

    next up is to actually execute the command.
    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    d8e95ec View commit details
    Browse the repository at this point in the history

  4. Rename flag to indicate experimental nature

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    122be7f View commit details
    Browse the repository at this point in the history

  5. Worker runs command, but without incremental mode and without redirec… 

    …ting stderr correctly.
    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    45da97c View commit details
    Browse the repository at this point in the history

  6. Sending command stderr to worker response output. works on Linux.

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    9540dc6 View commit details
    Browse the repository at this point in the history

  7. Use compilation mode as part of incremental dir path

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    3018ff9 View commit details
    Browse the repository at this point in the history

  8. When running the worker, the original executable should be marked as … 

    …an input via tools.
    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    ed6a0c3 View commit details
    Browse the repository at this point in the history

  9. Oof! This was traumatic. 

    A CodedOutputStream preserves its contents in some sense, possibly because of the use of EpsOutputStream and FileOutputStream beneath, which seem to keep buffers around. This was causing the original WorkResponse to be repeatedly written to stdout on future invocations of the worker, leading to Bazel not waiting around for the command to actually run.
Specifically, I believe what was happening was, the internal buffer had the initial WorkResponse. Every time Bazel sent a request, it tried to read the response and protobuf would just send the already available response right away.
    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    922efe2 View commit details
    Browse the repository at this point in the history

  10. Readability fixes and cleanup

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    a8da854 View commit details
    Browse the repository at this point in the history

  11. Encode target triple in incremental dir path 

    Consider the layout documented in
https://github.com/rust-lang/cargo/blob/58a961314437258065e23cb6316dfc121d96fb71/src/cargo/core/compiler/layout.rs#L50.
It relies on the incremental dir being in
`target/<triple>/{debug,release}/incremental`. With this change, Bazel
is creating a very similar structure.
    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    e02966b View commit details
    Browse the repository at this point in the history

  12. Add documentation on how to use the C++ worker, which is a lot easier

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    41f8049 View commit details
    Browse the repository at this point in the history

  13. refactor to move worker mode into function

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    7981f84 View commit details
    Browse the repository at this point in the history

  14. support for running standalone

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    83de9fe View commit details
    Browse the repository at this point in the history

  15. clean up indentation

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    c8165c7 View commit details
    Browse the repository at this point in the history

  16. default to false if attr is not present on rule

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    cfddfb1 View commit details
    Browse the repository at this point in the history

  17. buildifier fixes

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    66623d7 View commit details
    Browse the repository at this point in the history

  18. try adding CI job

    @nikhilm
    nikhilm committed Mar 31, 2021
    Configuration menu
    Copy the full SHA
    8b73681 View commit details
    Browse the repository at this point in the history