Make incr. comp. respect the -Ccodegen-units setting

[michaelwoerister]

TL;DR

Incremental compilation currently will always create 1-2 codegen units per source-level module, regardless of the -Ccodegen-units setting passed to the compiler. This is fine in the majority of cases but there is no way to control this behavior in cases where it produces too much overhead (see below for examples).

I propose to

• make the compiler honor the -Ccodegen-units setting, even when compiling with -Cincremental, and
• make the compiler default to a higher number of codegen units in case incr. comp. is enabled (256 instead of 16).

The -Ccodegen-units flag would retain exactly the same semantics it has in non-incremental mode, i.e. setting an upper bound for the number codegen units.

Why do I consider this a (possibly) major change? Because there is one case where the compiler changes behavior: If someone has explicitly set the number of codegen units. After this change, that setting will start to have an effect, leading to potentially higher compile times. Only one crate in the perf.rlo benchmark suite has such an explicit setting (clap-rs). I expect the fallout to be minor and harmless.

Also note that this opens up a whole new use case for incremental compilation: By setting -Ccodegen-units=1 (or -Ccodegen-units=16 as is the default right now), the compiler can make use of the incremental cache for all of the middle end while producing a binary that exhibits the same runtime performance as a non-incrementally built one.

Links and Details

I ran experiments for this in rust-lang/rust#67834 and the results look good:

• up to 30% compile time reduction for script-servo-debug
• up to 15% compile time reduction for style-servo-debug
• up to 8% compile time reduction for style-servo-opt

However, there are also cases that regress:

• patched incremental: debugging println in dependency in script-servo-opt regresses by 8% due to the lower cache granularity.
• clap-rs regresses by up to 34.7% because it has an explicit (and previously ignored) codegen-units setting in its Cargo.toml. That is easily fixable by clap-rs itself.

These regressions are acceptable, I think, especially because the user can easily regain the previous behavior by setting -Ccodegen-units=9999 (or some other number that is greater than the number of source-level modules times 2). Most crates, however, are well below the default setting of 256 codegen units and won't see any kind of changed behavior.

Mentors or Reviewers

The implementation should be straightforward so a reviewer would mostly need to sign off on making the -Ccodegen-units flag suddenly take an effect in incremental mode. @nikomatsakis & @pnkfelix as compiler team leads would be good candidates for that.

[Mark-Simulacrum]

Do we expect that we'll take the minimum codegen units between user specified and the amount of modules we have? For example hello world today presumably has 2 modules under incremental, will that still be the case by default and if codegen-units=200 is passed?

[michaelwoerister]

Yes, -Ccodegen-units today already only sets an upper limit. The same would be true after this change.

[pnkfelix]

You say at the outset that "Incremental compilation currently will always create 1-2 codegen units per source-level module, regardless of the -Ccodegen-units setting passed to the compiler."

Just to be absolutely clear: Your intention, with the change proposed here, is that -Ccodegen-units will denote the (upper-bound on the) number of codegen-units across the entire crate currently being compiled, correct?

• (For example, another interpretation of your words is that -Ccodegen-units would denote the number of codegen-units per source-level module, since today incrementatal always selects a value in the range 1-2 for that. This would be an admittedly strange interpretation given the other points you make.)

[michaelwoerister]

Just to be absolutely clear: Your intention, with the change proposed here, is that -Ccodegen-units will denote the (upper-bound on the) number of codegen-units across the entire crate currently being compiled, correct?

This is correct.

[nikomatsakis]

I brought this up in the compiler team triage meeting today -- I "second" this proposal and think we should move forward with it. Per the draft process we're talking about, it would be considered "accepted" in one week -- though I think this may be a case where we could forego the waiting period, since it seems unlikely to be controversial, and it's reversible.

[nikomatsakis]

This change has landed, closing.