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

Tracking issue for stabilizing "pipelined compilation" #60988

Closed
alexcrichton opened this issue May 20, 2019 · 12 comments · Fixed by #62766
Closed

Tracking issue for stabilizing "pipelined compilation" #60988

alexcrichton opened this issue May 20, 2019 · 12 comments · Fixed by #62766

Comments

@alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented May 20, 2019

This issue is intended to be an overall tracking issue for stabilizing the recently implemented feature of "pipelined compilation". This tracking issue spans both Cargo and rustc, as support currently lives in both. It also builds on top of a number of other tracking issues, since they're unstable features used to implement pipelined compilation!

For a bit of background, pipelined compilation has an initial set of notes about its implementation and initially had these tracking issues:

Currently today everything is implemented and available in nightly rustc. There is a thread on internals which is tasked with gaining user experience about pipelining and see the performance impact. A collation of these measurements is quite promising and I feel personally is convincing enough to push on stabilizing all the sub-features!

Known blockers for stabilization:

  • -Z emit-artifact-notifications - this is how Cargo learns the moment metadata is ready.
  • --json-rendered - this is how Cargo continues to provide pretty error messages.
  • There is no way to combine --message-format=short in Cargo with pipelining. The compiler currently has --error-format=json|short|human, but for this feature in Cargo we'll need something like --error-format=short-json or something like that. Basically some way to request that the rendered field in the JSON diagnostic is a "short message", not the long one.
  • Cargo pipelining is known to break with sccache, this should be investigated and fixed one way or another. (fixed in mozilla/sccache#441)
  • Pipelining fails with the RLS due to multiple --error-format flags (fixed in #62176)

Possible future work:

  • Should we game out what a "fully pipelined" story might look like? What if rustc blocked waiting for Cargo to tell it to proceed into the linking phase?
@Centril

This comment has been minimized.

Copy link
Member

@Centril Centril commented May 21, 2019

Reading the various issues and PRs, I got the impression that metadata is generated at earliest post static analysis, is that correct?

@nnethercote

This comment has been minimized.

Copy link
Contributor

@nnethercote nnethercote commented May 21, 2019

Yes: metadata is now generated right at the start of code generation, immediately after all the checking (typeck, borrowck, etc.) finish. This is the best place for the initial pipelining implementation.

One might argue that metadata generation should be moved earlier. That would help with successful builds, but slow down builds that trigger compile errors. Given that a lot of Rust compiler invocations end in compile errors during the normal development cycle, slowing down such invocations would be bad.

Another possibility is to generate metadata in parallel with checking. That might give the best of both worlds. But that's a bigger change.

@Centril

This comment has been minimized.

Copy link
Member

@Centril Centril commented May 21, 2019

One might argue that metadata generation should be moved earlier.

No no... ;) At the start of code-gen sounds good to me and we should be cautious with moving it too much earlier as that can mess with language design.

@oli-obk

This comment has been minimized.

Copy link
Contributor

@oli-obk oli-obk commented May 21, 2019

There is no way to combine --message-format=short in Cargo with pipelining

This is trivial to implement with --json-rendered. I can mentor this in #60987

@jsgf

This comment has been minimized.

Copy link
Contributor

@jsgf jsgf commented May 21, 2019

It might be useful to have multiple rendered forms in the json output.

Centril added a commit to Centril/rust that referenced this issue May 22, 2019
…hton

Make -Zemit-artifact-notifications also emit the artifact type

This is easier for tooling to handle than trying to reverse-engineer the type from the filename extension. The field name and value is intended to reflect the `--emit` command-line option.

Related issues rust-lang#60988 rust-lang#58465
cc @alexcrichton
Centril added a commit to Centril/rust that referenced this issue May 23, 2019
…hton

Make -Zemit-artifact-notifications also emit the artifact type

This is easier for tooling to handle than trying to reverse-engineer the type from the filename extension. The field name and value is intended to reflect the `--emit` command-line option.

Related issues rust-lang#60988 rust-lang#58465
cc @alexcrichton
@alexcrichton

This comment has been minimized.

Copy link
Member Author

@alexcrichton alexcrichton commented Jun 14, 2019

I've proposed stabilization of --json-rendered as well as -Z emit-artifact-notifications which, if stabilized, can allow Cargo to switch over to using these by default and stabilizing pipelined compilation.

@jsgf

This comment has been minimized.

Copy link
Contributor

@jsgf jsgf commented Jun 17, 2019

I'd like to make sure there's a stable way to make sure that --emit metadata on its own produces metadata suitable as an input to a next build stage. I'm fine with the plain cargo check metadata being incomplete if that allows it to be emitted a little faster, but I'd like to have the option of two-invocation pipelining without paying the cost of --emit metadata,link.

Context:

  • Invoking rustc twice is going to be easier to implement in Buck than full pipelining initially
  • Full pipelining is the goal, but Buck is not currently set up for it (I believe Bazel is in a similar position)
  • The extra CPU overhead of two rustc invocations is fine if it helps shorten the build critical path
  • But the cost of two complete builds is too much

(cc @bolinfest)

@alexcrichton

This comment has been minimized.

Copy link
Member Author

@alexcrichton alexcrichton commented Jun 18, 2019

I agree that it should be the case, but that seems like a separable stabilization issue from this which is enabling Cargo's internal pipelined compilation.

@Xanewok

This comment has been minimized.

Copy link
Member

@Xanewok Xanewok commented Jun 24, 2019

@alexcrichton IIUC it seems that pipelining always emits --error-format which currently seems to break the RLS due to error: Option 'error-format' given more than once (rust-lang/rls#1484).

The immediate fix is to filter the injected --error-format flag ourselves and continue with our --error-format=json flag (will that break the pipelining?). However, I imagine it'd be better to allow multiple occurrences of these with the later ones taking precedence not to reimplement command arguments parsing and filtering in rustc wrappers. Does that sound like a good idea?

@alexcrichton

This comment has been minimized.

Copy link
Member Author

@alexcrichton alexcrichton commented Jun 25, 2019

@Xanewok that sounds reasonable to me yeah with the last option taking precedence!

This was referenced Jun 25, 2019
bors added a commit that referenced this issue Jun 27, 2019
Update RLS

Merged PRs:
* fix(cmd): make clear_env_rust_log default to false (rust-lang/rls#1486)
  - Retain `RUST_LOG` in `rls --cli` mode
* Pass --file-lines to rustfmt only if specified (rust-lang/rls#1497)
  - Fix entire-file formatting when using external rustfmt (specified via `rustfmt_path` config)
* Ensure that --error-format is only passed once to `rustc` (rust-lang/rls#1500)
  - Unbreaks RLS when used together with Cargo [pipelining build](#60988) feature (@alexcrichton I'd consider this a stabilization blocker, mind adding it to the tracking issue for the record? 🙇 )
bors added a commit that referenced this issue Jun 27, 2019
Update RLS

Merged PRs:
* fix(cmd): make clear_env_rust_log default to false (rust-lang/rls#1486)
  - Retain `RUST_LOG` in `rls --cli` mode
* Pass --file-lines to rustfmt only if specified (rust-lang/rls#1497)
  - Fix entire-file formatting when using external rustfmt (specified via `rustfmt_path` config)
* Ensure that --error-format is only passed once to `rustc` (rust-lang/rls#1500)
  - Unbreaks RLS when used together with Cargo [pipelining build](#60988) feature (@alexcrichton I'd consider this a stabilization blocker, mind adding it to the tracking issue for the record? 🙇 )
Centril added a commit to Centril/rust that referenced this issue Jun 27, 2019
Update RLS

Merged PRs:
* fix(cmd): make clear_env_rust_log default to false (rust-lang/rls#1486)
  - Retain `RUST_LOG` in `rls --cli` mode
* Pass --file-lines to rustfmt only if specified (rust-lang/rls#1497)
  - Fix entire-file formatting when using external rustfmt (specified via `rustfmt_path` config)
* Ensure that --error-format is only passed once to `rustc` (rust-lang/rls#1500)
  - Unbreaks RLS when used together with Cargo [pipelining build](rust-lang#60988) feature (@alexcrichton I'd consider this a stabilization blocker, mind adding it to the tracking issue for the record? 🙇 )
bors added a commit that referenced this issue Jun 27, 2019
Update RLS

Merged PRs:
* fix(cmd): make clear_env_rust_log default to false (rust-lang/rls#1486)
  - Retain `RUST_LOG` in `rls --cli` mode
* Pass --file-lines to rustfmt only if specified (rust-lang/rls#1497)
  - Fix entire-file formatting when using external rustfmt (specified via `rustfmt_path` config)
* Ensure that --error-format is only passed once to `rustc` (rust-lang/rls#1500)
  - Unbreaks RLS when used together with Cargo [pipelining build](#60988) feature (@alexcrichton I'd consider this a stabilization blocker, mind adding it to the tracking issue for the record? 🙇 )
Centril added a commit to Centril/rust that referenced this issue Jun 27, 2019
Update RLS

Merged PRs:
* fix(cmd): make clear_env_rust_log default to false (rust-lang/rls#1486)
  - Retain `RUST_LOG` in `rls --cli` mode
* Pass --file-lines to rustfmt only if specified (rust-lang/rls#1497)
  - Fix entire-file formatting when using external rustfmt (specified via `rustfmt_path` config)
* Ensure that --error-format is only passed once to `rustc` (rust-lang/rls#1500)
  - Unbreaks RLS when used together with Cargo [pipelining build](rust-lang#60988) feature (@alexcrichton I'd consider this a stabilization blocker, mind adding it to the tracking issue for the record? 🙇 )
bors added a commit that referenced this issue Jun 27, 2019
Update RLS

Merged PRs:
* fix(cmd): make clear_env_rust_log default to false (rust-lang/rls#1486)
  - Retain `RUST_LOG` in `rls --cli` mode
* Pass --file-lines to rustfmt only if specified (rust-lang/rls#1497)
  - Fix entire-file formatting when using external rustfmt (specified via `rustfmt_path` config)
* Ensure that --error-format is only passed once to `rustc` (rust-lang/rls#1500)
  - Unbreaks RLS when used together with Cargo [pipelining build](#60988) feature (@alexcrichton I'd consider this a stabilization blocker, mind adding it to the tracking issue for the record? 🙇 )
bors added a commit that referenced this issue Jun 27, 2019
Update RLS

Merged PRs:
* fix(cmd): make clear_env_rust_log default to false (rust-lang/rls#1486)
  - Retain `RUST_LOG` in `rls --cli` mode
* Pass --file-lines to rustfmt only if specified (rust-lang/rls#1497)
  - Fix entire-file formatting when using external rustfmt (specified via `rustfmt_path` config)
* Ensure that --error-format is only passed once to `rustc` (rust-lang/rls#1500)
  - Unbreaks RLS when used together with Cargo [pipelining build](#60988) feature (@alexcrichton I'd consider this a stabilization blocker, mind adding it to the tracking issue for the record? 🙇 )
Centril added a commit to Centril/rust that referenced this issue Jun 27, 2019
Update RLS

Merged PRs:
* fix(cmd): make clear_env_rust_log default to false (rust-lang/rls#1486)
  - Retain `RUST_LOG` in `rls --cli` mode
* Pass --file-lines to rustfmt only if specified (rust-lang/rls#1497)
  - Fix entire-file formatting when using external rustfmt (specified via `rustfmt_path` config)
* Ensure that --error-format is only passed once to `rustc` (rust-lang/rls#1500)
  - Unbreaks RLS when used together with Cargo [pipelining build](rust-lang#60988) feature (@alexcrichton I'd consider this a stabilization blocker, mind adding it to the tracking issue for the record? 🙇 )
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 17, 2019
This commit stabilizes options in the compiler necessary for Cargo to
enable "pipelined compilation" by default. The concept of pipelined
compilation, how it's implemented, and what it means for rustc are
documented in rust-lang#60988. This PR is coupled with a PR against Cargo
(rust-lang/cargo#FIXME) which updates Cargo's support for pipelined
compliation to rustc, and also enables support by default in Cargo.
(note that the Cargo PR cannot land until this one against rustc lands).

The technical changes performed here were to stabilize the functionality
proposed in rust-lang#60419 and rust-lang#60987, the underlying pieces to enable pipelined
compilation support in Cargo. The issues have had some discussion during
stabilization, but the newly stabilized surface area here is:

* A new `--json` flag was added to the compiler.
* The `--json` flag can be passed multiple times.
* The value of the `--json` flag is a comma-separated list of
  directives.
* The `--json` flag cannot be combined with `--color`
* The `--json` flag must be combined with `--error-format=json`
* The acceptable list of directives to `--json` are:
  * `diagnostic-short` - the `rendered` field of diagnostics will have a
    "short" rendering matching `--error-format=short`
  * `diagnostic-rendered-ansi` - the `rendered` field of diagnostics
    will be colorized with ansi color codes embedded in the string field
  * `artifacts` - JSON blobs will be emitted for artifacts being emitted
    by the compiler

Closes rust-lang#60419
Closes rust-lang#60987
Closes rust-lang#60988
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 17, 2019
This commit stabilizes options in the compiler necessary for Cargo to
enable "pipelined compilation" by default. The concept of pipelined
compilation, how it's implemented, and what it means for rustc are
documented in rust-lang#60988. This PR is coupled with a PR against Cargo
(rust-lang/cargo#FIXME) which updates Cargo's support for pipelined
compliation to rustc, and also enables support by default in Cargo.
(note that the Cargo PR cannot land until this one against rustc lands).

The technical changes performed here were to stabilize the functionality
proposed in rust-lang#60419 and rust-lang#60987, the underlying pieces to enable pipelined
compilation support in Cargo. The issues have had some discussion during
stabilization, but the newly stabilized surface area here is:

* A new `--json` flag was added to the compiler.
* The `--json` flag can be passed multiple times.
* The value of the `--json` flag is a comma-separated list of
  directives.
* The `--json` flag cannot be combined with `--color`
* The `--json` flag must be combined with `--error-format=json`
* The acceptable list of directives to `--json` are:
  * `diagnostic-short` - the `rendered` field of diagnostics will have a
    "short" rendering matching `--error-format=short`
  * `diagnostic-rendered-ansi` - the `rendered` field of diagnostics
    will be colorized with ansi color codes embedded in the string field
  * `artifacts` - JSON blobs will be emitted for artifacts being emitted
    by the compiler

The unstable `-Z emit-artifact-notifications` and `--json-rendered`
flags have also been removed during this commit as well.

Closes rust-lang#60419
Closes rust-lang#60987
Closes rust-lang#60988
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 17, 2019
This commit stabilizes options in the compiler necessary for Cargo to
enable "pipelined compilation" by default. The concept of pipelined
compilation, how it's implemented, and what it means for rustc are
documented in rust-lang#60988. This PR is coupled with a PR against Cargo
(rust-lang/cargo#7143) which updates Cargo's support for pipelined
compliation to rustc, and also enables support by default in Cargo.
(note that the Cargo PR cannot land until this one against rustc lands).

The technical changes performed here were to stabilize the functionality
proposed in rust-lang#60419 and rust-lang#60987, the underlying pieces to enable pipelined
compilation support in Cargo. The issues have had some discussion during
stabilization, but the newly stabilized surface area here is:

* A new `--json` flag was added to the compiler.
* The `--json` flag can be passed multiple times.
* The value of the `--json` flag is a comma-separated list of
  directives.
* The `--json` flag cannot be combined with `--color`
* The `--json` flag must be combined with `--error-format=json`
* The acceptable list of directives to `--json` are:
  * `diagnostic-short` - the `rendered` field of diagnostics will have a
    "short" rendering matching `--error-format=short`
  * `diagnostic-rendered-ansi` - the `rendered` field of diagnostics
    will be colorized with ansi color codes embedded in the string field
  * `artifacts` - JSON blobs will be emitted for artifacts being emitted
    by the compiler

The unstable `-Z emit-artifact-notifications` and `--json-rendered`
flags have also been removed during this commit as well.

Closes rust-lang#60419
Closes rust-lang#60987
Closes rust-lang#60988
@alexcrichton

This comment has been minimized.

Copy link
Member Author

@alexcrichton alexcrichton commented Jul 17, 2019

The two blocking issues are now in FCP:

and I have posted two PRs to stabilize the implementation:

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 18, 2019
This commit stabilizes options in the compiler necessary for Cargo to
enable "pipelined compilation" by default. The concept of pipelined
compilation, how it's implemented, and what it means for rustc are
documented in rust-lang#60988. This PR is coupled with a PR against Cargo
(rust-lang/cargo#7143) which updates Cargo's support for pipelined
compliation to rustc, and also enables support by default in Cargo.
(note that the Cargo PR cannot land until this one against rustc lands).

The technical changes performed here were to stabilize the functionality
proposed in rust-lang#60419 and rust-lang#60987, the underlying pieces to enable pipelined
compilation support in Cargo. The issues have had some discussion during
stabilization, but the newly stabilized surface area here is:

* A new `--json` flag was added to the compiler.
* The `--json` flag can be passed multiple times.
* The value of the `--json` flag is a comma-separated list of
  directives.
* The `--json` flag cannot be combined with `--color`
* The `--json` flag must be combined with `--error-format=json`
* The acceptable list of directives to `--json` are:
  * `diagnostic-short` - the `rendered` field of diagnostics will have a
    "short" rendering matching `--error-format=short`
  * `diagnostic-rendered-ansi` - the `rendered` field of diagnostics
    will be colorized with ansi color codes embedded in the string field
  * `artifacts` - JSON blobs will be emitted for artifacts being emitted
    by the compiler

The unstable `-Z emit-artifact-notifications` and `--json-rendered`
flags have also been removed during this commit as well.

Closes rust-lang#60419
Closes rust-lang#60987
Closes rust-lang#60988
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 24, 2019
This commit stabilizes options in the compiler necessary for Cargo to
enable "pipelined compilation" by default. The concept of pipelined
compilation, how it's implemented, and what it means for rustc are
documented in rust-lang#60988. This PR is coupled with a PR against Cargo
(rust-lang/cargo#7143) which updates Cargo's support for pipelined
compliation to rustc, and also enables support by default in Cargo.
(note that the Cargo PR cannot land until this one against rustc lands).

The technical changes performed here were to stabilize the functionality
proposed in rust-lang#60419 and rust-lang#60987, the underlying pieces to enable pipelined
compilation support in Cargo. The issues have had some discussion during
stabilization, but the newly stabilized surface area here is:

* A new `--json` flag was added to the compiler.
* The `--json` flag can be passed multiple times.
* The value of the `--json` flag is a comma-separated list of
  directives.
* The `--json` flag cannot be combined with `--color`
* The `--json` flag must be combined with `--error-format=json`
* The acceptable list of directives to `--json` are:
  * `diagnostic-short` - the `rendered` field of diagnostics will have a
    "short" rendering matching `--error-format=short`
  * `diagnostic-rendered-ansi` - the `rendered` field of diagnostics
    will be colorized with ansi color codes embedded in the string field
  * `artifacts` - JSON blobs will be emitted for artifacts being emitted
    by the compiler

The unstable `-Z emit-artifact-notifications` and `--json-rendered`
flags have also been removed during this commit as well.

Closes rust-lang#60419
Closes rust-lang#60987
Closes rust-lang#60988
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 24, 2019
This commit stabilizes options in the compiler necessary for Cargo to
enable "pipelined compilation" by default. The concept of pipelined
compilation, how it's implemented, and what it means for rustc are
documented in rust-lang#60988. This PR is coupled with a PR against Cargo
(rust-lang/cargo#7143) which updates Cargo's support for pipelined
compliation to rustc, and also enables support by default in Cargo.
(note that the Cargo PR cannot land until this one against rustc lands).

The technical changes performed here were to stabilize the functionality
proposed in rust-lang#60419 and rust-lang#60987, the underlying pieces to enable pipelined
compilation support in Cargo. The issues have had some discussion during
stabilization, but the newly stabilized surface area here is:

* A new `--json` flag was added to the compiler.
* The `--json` flag can be passed multiple times.
* The value of the `--json` flag is a comma-separated list of
  directives.
* The `--json` flag cannot be combined with `--color`
* The `--json` flag must be combined with `--error-format=json`
* The acceptable list of directives to `--json` are:
  * `diagnostic-short` - the `rendered` field of diagnostics will have a
    "short" rendering matching `--error-format=short`
  * `diagnostic-rendered-ansi` - the `rendered` field of diagnostics
    will be colorized with ansi color codes embedded in the string field
  * `artifacts` - JSON blobs will be emitted for artifacts being emitted
    by the compiler

The unstable `-Z emit-artifact-notifications` and `--json-rendered`
flags have also been removed during this commit as well.

Closes rust-lang#60419
Closes rust-lang#60987
Closes rust-lang#60988
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 26, 2019
This commit stabilizes options in the compiler necessary for Cargo to
enable "pipelined compilation" by default. The concept of pipelined
compilation, how it's implemented, and what it means for rustc are
documented in rust-lang#60988. This PR is coupled with a PR against Cargo
(rust-lang/cargo#7143) which updates Cargo's support for pipelined
compliation to rustc, and also enables support by default in Cargo.
(note that the Cargo PR cannot land until this one against rustc lands).

The technical changes performed here were to stabilize the functionality
proposed in rust-lang#60419 and rust-lang#60987, the underlying pieces to enable pipelined
compilation support in Cargo. The issues have had some discussion during
stabilization, but the newly stabilized surface area here is:

* A new `--json` flag was added to the compiler.
* The `--json` flag can be passed multiple times.
* The value of the `--json` flag is a comma-separated list of
  directives.
* The `--json` flag cannot be combined with `--color`
* The `--json` flag must be combined with `--error-format=json`
* The acceptable list of directives to `--json` are:
  * `diagnostic-short` - the `rendered` field of diagnostics will have a
    "short" rendering matching `--error-format=short`
  * `diagnostic-rendered-ansi` - the `rendered` field of diagnostics
    will be colorized with ansi color codes embedded in the string field
  * `artifacts` - JSON blobs will be emitted for artifacts being emitted
    by the compiler

The unstable `-Z emit-artifact-notifications` and `--json-rendered`
flags have also been removed during this commit as well.

Closes rust-lang#60419
Closes rust-lang#60987
Closes rust-lang#60988
bors added a commit that referenced this issue Jul 30, 2019
…r=oli-obk

rustc: Stabilize options for pipelined compilation

This commit stabilizes options in the compiler necessary for Cargo to
enable "pipelined compilation" by default. The concept of pipelined
compilation, how it's implemented, and what it means for rustc are
documented in #60988. This PR is coupled with a PR against Cargo
(rust-lang/cargo#7143) which updates Cargo's support for pipelined
compliation to rustc, and also enables support by default in Cargo.
(note that the Cargo PR cannot land until this one against rustc lands).

The technical changes performed here were to stabilize the functionality
proposed in #60419 and #60987, the underlying pieces to enable pipelined
compilation support in Cargo. The issues have had some discussion during
stabilization, but the newly stabilized surface area here is:

* A new `--json` flag was added to the compiler.
* The `--json` flag can be passed multiple times.
* The value of the `--json` flag is a comma-separated list of
  directives.
* The `--json` flag cannot be combined with `--color`
* The `--json` flag must be combined with `--error-format=json`
* The acceptable list of directives to `--json` are:
  * `diagnostic-short` - the `rendered` field of diagnostics will have a
    "short" rendering matching `--error-format=short`
  * `diagnostic-rendered-ansi` - the `rendered` field of diagnostics
    will be colorized with ansi color codes embedded in the string field
  * `artifacts` - JSON blobs will be emitted for artifacts being emitted
    by the compiler

The unstable `-Z emit-artifact-notifications` and `--json-rendered`
flags have also been removed during this commit as well.

Closes #60419
Closes #60987
Closes #60988
@bors bors closed this in #62766 Jul 30, 2019
@umanwizard

This comment has been minimized.

Copy link

@umanwizard umanwizard commented Aug 9, 2019

The NOTES.md link in the description is dead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.