Avoid many CrateConfig clones. #37161
Conversation
This commit changes `ExtCtx::cfg()` so it returns a `CrateConfig` reference instead of a clone. As a result, it also changes all of the `cfg()` callsites to explicitly clone... except one, because the commit also changes `macro_parser::parse()` to take `&CrateConfig`. This is good, because that function can be hot, and `CrateConfig` is expensive to clone. This change almost halves the number of heap allocations done by rustc for `html5ever` in rustc-benchmarks suite, which makes compilation 1.20x faster.
|
If we can't use references in some cases but the data is always immutable, why not wrap it in |
|
I tried making it a |
|
I mean, either |
I don't understand what you mean by this. Can you elaborate? Thanks. |
|
I mean that there's still many clones and some of them may contribute to expansion times in other crates. |
Premature optimization, don't let perfect be the enemy of good, etc, etc. Let's see what @nrc thinks. |
|
Hey I thought the motto of Rust was "Don't let 'good enough' be the enemy of 'perfect'" 😆 |
I know you are at least partly joking, but I will respond seriously: an uncompromising attitude like that makes sense for aspects of Rust's design as a language -- e.g. no compromise on memory safety -- but I don't think the same attitude is necessarily relevant or helpful when it comes to the compiler implementation. |
|
📌 Commit 029dcee has been approved by |
|
@nrc This PR adds clone calls all over the place, using |
Avoid many CrateConfig clones. This commit changes `ExtCtx::cfg()` so it returns a `CrateConfig` reference instead of a clone. As a result, it also changes all of the `cfg()` callsites to explicitly clone... except one, because the commit also changes `macro_parser::parse()` to take `&CrateConfig`. This is good, because that function can be hot, and `CrateConfig` is expensive to clone. This change almost halves the number of heap allocations done by rustc for `html5ever` in rustc-benchmarks suite, which makes compilation 1.20x faster. r? @nrc
Avoid many CrateConfig clones. This commit changes `ExtCtx::cfg()` so it returns a `CrateConfig` reference instead of a clone. As a result, it also changes all of the `cfg()` callsites to explicitly clone... except one, because the commit also changes `macro_parser::parse()` to take `&CrateConfig`. This is good, because that function can be hot, and `CrateConfig` is expensive to clone. This change almost halves the number of heap allocations done by rustc for `html5ever` in rustc-benchmarks suite, which makes compilation 1.20x faster. r? @nrc
@jseyfried ended up removing all the |
This commit changes
ExtCtx::cfg()so it returns aCrateConfigreference instead of a clone. As a result, it also changes all of the
cfg()callsites to explicitly clone... except one, because the commitalso changes
macro_parser::parse()to take&CrateConfig. This isgood, because that function can be hot, and
CrateConfigis expensiveto clone.
This change almost halves the number of heap allocations done by rustc
for
html5everin rustc-benchmarks suite, which makes compilation 1.20xfaster.
r? @nrc