Take command line arguments into account when doing incremental compilation #33727
Comments
|
I've taken an initial survey of all command line arguments for rustc. Feel free to correct any mistakes, as I'm sure there will be a few. Options marked with ✔️ affect incr. comp.: When those options are added/removed between runs, everything should be recompiled.
Sorry for this wall of text, rustc has more options than I remember 😅 |
|
@jonas-schievink Wow, thanks a lot for this listing! That's very impressive |
|
OK, I see two basic approaches for implementing this:
Advantages of the Conservative Approach
Disadvantages of the Conservative Approach
Advantages of the DepGraph-based Approach
Disadvantages of the DepGraph-based Approach
Initially I thought that the DepGraph-based approach would be the natural way to do it, but now I'm thinking that it might be better to go the conservative route, at least in the beginning. It reduces the risk for bugs and, at least initially, should have no negative effect on performance. I'd be interested in what others think here. Conservative Approach: Implementation
With these categories defined, run the following algorithm:
I was also thinking about sub-dividing the incr. comp. directory, depending on given set of |
|
Makes sense to me to take the conservative approach to start out at least, and the proposed strategy here seems reasonable to me. I agree that Cargo should probably deal with the separate incremental compilation directories, it'll just have one for debug/release mode. Additionally in practice the arguments shouldn't change much unless the cache actually needs to be invalidated because Cargo'll pass the same arguments, so the conservative approach I think will pan out well enough. One question I'd have is how we protect against new flags being added to the compiler? Would we ensure that all options are tagged with relevant/irrelevant to incremental compilation, or would we just have whitelist of "irrelevant" options you'd have to add to if it's relevant? |
|
I prototyped the DepGraph approach the other day and there I extended the options! macro in sess::config to require that one specifies the DepNode that the option should be mapped to. Something similar (but simpler) can also be done here, I guess. A whitelist seems good too. On July 21, 2016 7:42:41 PM GMT+02:00, Alex Crichton notifications@github.com wrote:
|
|
@michaelwoerister so the simplest thing I can imagine is just to hash the options and encode that in the serialized state. When we re-load the state, we could compare against our current options and just choose to ignore everything if the options changed. But I guess if you've worked through some of the dep-graph approach that'd be that much better. |
|
@nikomatsakis Prototyping the DepGraph approach was mostly to get a feel for what it would look like -- and subsequently lead me to prefer the simpler approach. What you describe is pretty much how I intend to implement it, just with filtering out those options before hashing that should not have any influence. |
…sakis Take commandline arguments into account for incr. comp. Implements the conservative strategy described in #33727. From now one, every time a new commandline option is added, one has to specify if it influences the incremental compilation cache. I've tried to implement this as automatic as possible: One just has to added either the `[TRACKED]` or the `[UNTRACKED]` marker next to the field. The `Options`, `CodegenOptions`, and `DebuggingOptions` definitions in `session::config` show plenty of examples. The PR removes some cruft from `session::config::Options`, mostly unnecessary copies of flags also present in `DebuggingOptions` or `CodeGenOptions` in the same struct. One notable removal is the `cfg` field that contained the values passed via `--cfg` commandline arguments. I chose to remove it because (1) its content is only a subset of what later is stored in `hir::Crate::config` and it's pretty likely that reading the cfgs from `Options` would not be what you wanted, and (2) we could not incorporate it into the dep-tracking hash of the `Options` struct because of how the test framework works, leaving us with a piece of untracked but vital data. It is now recommended (just as before) to access the crate config via the `krate()` method in the HIR map. Because the `cfg` field is not present in the `Options` struct any more, some methods in the `CompilerCalls` trait now take the crate config as an explicit parameter -- which might constitute a breaking change for plugin authors.
…sakis Take commandline arguments into account for incr. comp. Implements the conservative strategy described in #33727. From now one, every time a new commandline option is added, one has to specify if it influences the incremental compilation cache. I've tried to implement this as automatic as possible: One just has to added either the `[TRACKED]` or the `[UNTRACKED]` marker next to the field. The `Options`, `CodegenOptions`, and `DebuggingOptions` definitions in `session::config` show plenty of examples. The PR removes some cruft from `session::config::Options`, mostly unnecessary copies of flags also present in `DebuggingOptions` or `CodeGenOptions` in the same struct. One notable removal is the `cfg` field that contained the values passed via `--cfg` commandline arguments. I chose to remove it because (1) its content is only a subset of what later is stored in `hir::Crate::config` and it's pretty likely that reading the cfgs from `Options` would not be what you wanted, and (2) we could not incorporate it into the dep-tracking hash of the `Options` struct because of how the test framework works, leaving us with a piece of untracked but vital data. It is now recommended (just as before) to access the crate config via the `krate()` method in the HIR map. Because the `cfg` field is not present in the `Options` struct any more, some methods in the `CompilerCalls` trait now take the crate config as an explicit parameter -- which might constitute a breaking change for plugin authors.
…sakis Take commandline arguments into account for incr. comp. Implements the conservative strategy described in #33727. From now one, every time a new commandline option is added, one has to specify if it influences the incremental compilation cache. I've tried to implement this as automatic as possible: One just has to added either the `[TRACKED]` or the `[UNTRACKED]` marker next to the field. The `Options`, `CodegenOptions`, and `DebuggingOptions` definitions in `session::config` show plenty of examples. The PR removes some cruft from `session::config::Options`, mostly unnecessary copies of flags also present in `DebuggingOptions` or `CodeGenOptions` in the same struct. One notable removal is the `cfg` field that contained the values passed via `--cfg` commandline arguments. I chose to remove it because (1) its content is only a subset of what later is stored in `hir::Crate::config` and it's pretty likely that reading the cfgs from `Options` would not be what you wanted, and (2) we could not incorporate it into the dep-tracking hash of the `Options` struct because of how the test framework works, leaving us with a piece of untracked but vital data. It is now recommended (just as before) to access the crate config via the `krate()` method in the HIR map. Because the `cfg` field is not present in the `Options` struct any more, some methods in the `CompilerCalls` trait now take the crate config as an explicit parameter -- which might constitute a breaking change for plugin authors.
|
#35340 has landed, closing |
…sakis Take commandline arguments into account for incr. comp. Implements the conservative strategy described in rust-lang/rust#33727. From now one, every time a new commandline option is added, one has to specify if it influences the incremental compilation cache. I've tried to implement this as automatic as possible: One just has to added either the `[TRACKED]` or the `[UNTRACKED]` marker next to the field. The `Options`, `CodegenOptions`, and `DebuggingOptions` definitions in `session::config` show plenty of examples. The PR removes some cruft from `session::config::Options`, mostly unnecessary copies of flags also present in `DebuggingOptions` or `CodeGenOptions` in the same struct. One notable removal is the `cfg` field that contained the values passed via `--cfg` commandline arguments. I chose to remove it because (1) its content is only a subset of what later is stored in `hir::Crate::config` and it's pretty likely that reading the cfgs from `Options` would not be what you wanted, and (2) we could not incorporate it into the dep-tracking hash of the `Options` struct because of how the test framework works, leaving us with a piece of untracked but vital data. It is now recommended (just as before) to access the crate config via the `krate()` method in the HIR map. Because the `cfg` field is not present in the `Options` struct any more, some methods in the `CompilerCalls` trait now take the crate config as an explicit parameter -- which might constitute a breaking change for plugin authors.
…sakis Take commandline arguments into account for incr. comp. Implements the conservative strategy described in rust-lang/rust#33727. From now one, every time a new commandline option is added, one has to specify if it influences the incremental compilation cache. I've tried to implement this as automatic as possible: One just has to added either the `[TRACKED]` or the `[UNTRACKED]` marker next to the field. The `Options`, `CodegenOptions`, and `DebuggingOptions` definitions in `session::config` show plenty of examples. The PR removes some cruft from `session::config::Options`, mostly unnecessary copies of flags also present in `DebuggingOptions` or `CodeGenOptions` in the same struct. One notable removal is the `cfg` field that contained the values passed via `--cfg` commandline arguments. I chose to remove it because (1) its content is only a subset of what later is stored in `hir::Crate::config` and it's pretty likely that reading the cfgs from `Options` would not be what you wanted, and (2) we could not incorporate it into the dep-tracking hash of the `Options` struct because of how the test framework works, leaving us with a piece of untracked but vital data. It is now recommended (just as before) to access the crate config via the `krate()` method in the HIR map. Because the `cfg` field is not present in the `Options` struct any more, some methods in the `CompilerCalls` trait now take the crate config as an explicit parameter -- which might constitute a breaking change for plugin authors.
Since command line arguments to the compiler influence the code generated, changing them should also purge affected artifacts from the cache. It might be a good idea to treat them just like HIR or Metadata nodes and thus make them part of the regular dep-graph.
cfgsettings are a special case. Since they will affect the HIR being generated, they need not be considered for incr. comp. However, it might make sense to have separate caches for different sets ofcfgflags, iff we want to support these being changed frequently without having to recompile a lot. Although that might be a negligible use case.A first step to implementing this is probably taking a survey of all command line options and seeing what they might affect.
The text was updated successfully, but these errors were encountered: