rustdoc: Only analyze head of self type when deciding impl inlining - #159854
Conversation
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rustdoc: Only analyze head of self type when deciding impl inlining
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (b8110ca): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. @bors rollup=never rustc-perf Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -5.9%, secondary -8.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -11.5%, secondary -23.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 488.104s -> 487.819s (-0.06%) |
We only care about whether the self type is a generic or an item (inlined) in the current crate, so we don't actually need to compute the param_env, which is expensive when done to every external impl.
|
r? @notriddle rustbot has assigned @notriddle. Use Why was this reviewer chosen?The reviewer was selected based on:
|
69b025c to
a1136eb
Compare
If it doesn't have any trade-offs that we would care about, please do send a PR! Building docs is indeed a non-trivial bottleneck for our CI. |
This feature is unstable but will be stabilized soon, and this is a good way of dogfooding it to make sure it works properly. It should have no effect on the generated docs, but it provides a significant speedup. For example, I measure a 3x speedup locally (3m 11s -> 1m 1s) -- note that this is with the latest rustdoc perf improvements (#159854).
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test for e59475a failed: CI. Failed job:
|
|
Argh. Appears spurious AFAICT. I don't even see an error in the logs, weirdly... @bors retry spurious> |
|
Just to be sure |
This comment has been minimized.
This comment has been minimized.
rustdoc: Only analyze head of self type when deciding impl inlining try-job: dist-x86_64-solaris
This comment has been minimized.
This comment has been minimized.
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 26ae60a (parent) -> 701a651 (this PR) Test differencesShow 6 test diffsStage 1
Stage 2
Additionally, 4 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 701a6513a48eac30d49110ba06187648b7553622 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (701a651): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -6.5%, secondary -10.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -12.1%, secondary -25.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 489.651s -> 491.747s (0.43%) |
…obzol,jieyouxu bootstrap: Enable rustdoc mergeable CCI for std and internal docs Takes a different approach to #160098, where the internal docs are merged by bootstrap directly invoking rustdoc. This requires bootstrap to gather the list of metadata directories by inspecting cargo's fingerprint files (which aren't stable). The first commit is written by @camelid, but I wrote the other two. This feature is needed because: 1. The search index (that powers [web-based search](https://doc.rust-lang.org/nightly/nightly-rustc/?search=ty%20-%3E%20rustdoc%3A%3Atype)) needs to contain all of the crates in the nightly-rustc project. In particular, I'd prefer if it contained Clippy, Rustdoc, and Rustc, since those crates share type checker stuff and the ability to search all three at once is convenient. 2. For every crate that rustdoc *currently* documents, it has to load the search index from the doc output dir, and rebuild the search index with the new crate added to it. Loading the search index requires $O(\text{crates})$ work, so doing it once for every crate means we're doing $O(\text{crates}^2)$ work overall. 3. It would be more efficient, *instead*, if each crate wrote its data separately, and then the final search index was generated at the end by merging them all at once. Obviously, this would make the work linear instead of quadratic. For the record, Hoogle and Sherlodoc have a similar index-generating step. 4. We call this "Mergeable Cross-Crate-Information." Cargo stores it in the build directory, and supplies it to Rustdoc in a separate phase that runs after everything else. When we eventually stabilize this feature, it will be invisible to (most) end users. `cargo doc` will just be faster. 5. So, in order for crates to share their cross-crate info, we need them to share a build directory. 6. Tools, like Rustdoc and Cargo, don't normally share a build directory with Rustc. 7. To make them share a build directory while generating documentation, without forcing them to share a build directory while compiling, I added a new mode. --- This rustdoc feature is unstable but will be stabilized soon, and this is a good way of dogfooding it to make sure it works properly. It should have no effect on the generated docs, but it provides a significant speedup. For example, I measure a 3x speedup locally (3m 11s -> 1m 1s) for `x doc src/tools` -- note that this is with the latest rustdoc perf improvements (#159854). r? @Kobzol
…obzol,jieyouxu bootstrap: Enable rustdoc mergeable CCI for std and internal docs Takes a different approach to rust-lang/rust#160098, where the internal docs are merged by bootstrap directly invoking rustdoc. This requires bootstrap to gather the list of metadata directories by inspecting cargo's fingerprint files (which aren't stable). The first commit is written by @camelid, but I wrote the other two. This feature is needed because: 1. The search index (that powers [web-based search](https://doc.rust-lang.org/nightly/nightly-rustc/?search=ty%20-%3E%20rustdoc%3A%3Atype)) needs to contain all of the crates in the nightly-rustc project. In particular, I'd prefer if it contained Clippy, Rustdoc, and Rustc, since those crates share type checker stuff and the ability to search all three at once is convenient. 2. For every crate that rustdoc *currently* documents, it has to load the search index from the doc output dir, and rebuild the search index with the new crate added to it. Loading the search index requires $O(\text{crates})$ work, so doing it once for every crate means we're doing $O(\text{crates}^2)$ work overall. 3. It would be more efficient, *instead*, if each crate wrote its data separately, and then the final search index was generated at the end by merging them all at once. Obviously, this would make the work linear instead of quadratic. For the record, Hoogle and Sherlodoc have a similar index-generating step. 4. We call this "Mergeable Cross-Crate-Information." Cargo stores it in the build directory, and supplies it to Rustdoc in a separate phase that runs after everything else. When we eventually stabilize this feature, it will be invisible to (most) end users. `cargo doc` will just be faster. 5. So, in order for crates to share their cross-crate info, we need them to share a build directory. 6. Tools, like Rustdoc and Cargo, don't normally share a build directory with Rustc. 7. To make them share a build directory while generating documentation, without forcing them to share a build directory while compiling, I added a new mode. --- This rustdoc feature is unstable but will be stabilized soon, and this is a good way of dogfooding it to make sure it works properly. It should have no effect on the generated docs, but it provides a significant speedup. For example, I measure a 3x speedup locally (3m 11s -> 1m 1s) for `x doc src/tools` -- note that this is with the latest rustdoc perf improvements (rust-lang/rust#159854). r? @Kobzol
…obzol,jieyouxu bootstrap: Enable rustdoc mergeable CCI for std and internal docs Takes a different approach to rust-lang/rust#160098, where the internal docs are merged by bootstrap directly invoking rustdoc. This requires bootstrap to gather the list of metadata directories by inspecting cargo's fingerprint files (which aren't stable). The first commit is written by @camelid, but I wrote the other two. This feature is needed because: 1. The search index (that powers [web-based search](https://doc.rust-lang.org/nightly/nightly-rustc/?search=ty%20-%3E%20rustdoc%3A%3Atype)) needs to contain all of the crates in the nightly-rustc project. In particular, I'd prefer if it contained Clippy, Rustdoc, and Rustc, since those crates share type checker stuff and the ability to search all three at once is convenient. 2. For every crate that rustdoc *currently* documents, it has to load the search index from the doc output dir, and rebuild the search index with the new crate added to it. Loading the search index requires $O(\text{crates})$ work, so doing it once for every crate means we're doing $O(\text{crates}^2)$ work overall. 3. It would be more efficient, *instead*, if each crate wrote its data separately, and then the final search index was generated at the end by merging them all at once. Obviously, this would make the work linear instead of quadratic. For the record, Hoogle and Sherlodoc have a similar index-generating step. 4. We call this "Mergeable Cross-Crate-Information." Cargo stores it in the build directory, and supplies it to Rustdoc in a separate phase that runs after everything else. When we eventually stabilize this feature, it will be invisible to (most) end users. `cargo doc` will just be faster. 5. So, in order for crates to share their cross-crate info, we need them to share a build directory. 6. Tools, like Rustdoc and Cargo, don't normally share a build directory with Rustc. 7. To make them share a build directory while generating documentation, without forcing them to share a build directory while compiling, I added a new mode. --- This rustdoc feature is unstable but will be stabilized soon, and this is a good way of dogfooding it to make sure it works properly. It should have no effect on the generated docs, but it provides a significant speedup. For example, I measure a 3x speedup locally (3m 11s -> 1m 1s) for `x doc src/tools` -- note that this is with the latest rustdoc perf improvements (rust-lang/rust#159854). r? @Kobzol
…obzol,jieyouxu bootstrap: Enable rustdoc mergeable CCI for std and internal docs Takes a different approach to rust-lang/rust#160098, where the internal docs are merged by bootstrap directly invoking rustdoc. This requires bootstrap to gather the list of metadata directories by inspecting cargo's fingerprint files (which aren't stable). The first commit is written by @camelid, but I wrote the other two. This feature is needed because: 1. The search index (that powers [web-based search](https://doc.rust-lang.org/nightly/nightly-rustc/?search=ty%20-%3E%20rustdoc%3A%3Atype)) needs to contain all of the crates in the nightly-rustc project. In particular, I'd prefer if it contained Clippy, Rustdoc, and Rustc, since those crates share type checker stuff and the ability to search all three at once is convenient. 2. For every crate that rustdoc *currently* documents, it has to load the search index from the doc output dir, and rebuild the search index with the new crate added to it. Loading the search index requires $O(\text{crates})$ work, so doing it once for every crate means we're doing $O(\text{crates}^2)$ work overall. 3. It would be more efficient, *instead*, if each crate wrote its data separately, and then the final search index was generated at the end by merging them all at once. Obviously, this would make the work linear instead of quadratic. For the record, Hoogle and Sherlodoc have a similar index-generating step. 4. We call this "Mergeable Cross-Crate-Information." Cargo stores it in the build directory, and supplies it to Rustdoc in a separate phase that runs after everything else. When we eventually stabilize this feature, it will be invisible to (most) end users. `cargo doc` will just be faster. 5. So, in order for crates to share their cross-crate info, we need them to share a build directory. 6. Tools, like Rustdoc and Cargo, don't normally share a build directory with Rustc. 7. To make them share a build directory while generating documentation, without forcing them to share a build directory while compiling, I added a new mode. --- This rustdoc feature is unstable but will be stabilized soon, and this is a good way of dogfooding it to make sure it works properly. It should have no effect on the generated docs, but it provides a significant speedup. For example, I measure a 3x speedup locally (3m 11s -> 1m 1s) for `x doc src/tools` -- note that this is with the latest rustdoc perf improvements (rust-lang/rust#159854). r? @Kobzol
…obzol,jieyouxu bootstrap: Enable rustdoc mergeable CCI for std and internal docs Takes a different approach to rust-lang/rust#160098, where the internal docs are merged by bootstrap directly invoking rustdoc. This requires bootstrap to gather the list of metadata directories by inspecting cargo's fingerprint files (which aren't stable). The first commit is written by @camelid, but I wrote the other two. This feature is needed because: 1. The search index (that powers [web-based search](https://doc.rust-lang.org/nightly/nightly-rustc/?search=ty%20-%3E%20rustdoc%3A%3Atype)) needs to contain all of the crates in the nightly-rustc project. In particular, I'd prefer if it contained Clippy, Rustdoc, and Rustc, since those crates share type checker stuff and the ability to search all three at once is convenient. 2. For every crate that rustdoc *currently* documents, it has to load the search index from the doc output dir, and rebuild the search index with the new crate added to it. Loading the search index requires $O(\text{crates})$ work, so doing it once for every crate means we're doing $O(\text{crates}^2)$ work overall. 3. It would be more efficient, *instead*, if each crate wrote its data separately, and then the final search index was generated at the end by merging them all at once. Obviously, this would make the work linear instead of quadratic. For the record, Hoogle and Sherlodoc have a similar index-generating step. 4. We call this "Mergeable Cross-Crate-Information." Cargo stores it in the build directory, and supplies it to Rustdoc in a separate phase that runs after everything else. When we eventually stabilize this feature, it will be invisible to (most) end users. `cargo doc` will just be faster. 5. So, in order for crates to share their cross-crate info, we need them to share a build directory. 6. Tools, like Rustdoc and Cargo, don't normally share a build directory with Rustc. 7. To make them share a build directory while generating documentation, without forcing them to share a build directory while compiling, I added a new mode. --- This rustdoc feature is unstable but will be stabilized soon, and this is a good way of dogfooding it to make sure it works properly. It should have no effect on the generated docs, but it provides a significant speedup. For example, I measure a 3x speedup locally (3m 11s -> 1m 1s) for `x doc src/tools` -- note that this is with the latest rustdoc perf improvements (rust-lang/rust#159854). r? @Kobzol
View all comments
We only care about whether the self type is a generic or an item (inlined) in
the current crate, so we don't actually need to compute the param_env, which is
expensive when done to every external impl. This PR avoids computing the
param_env until we actually decide to inline the impl. Moreover, it adds
specialized cleaning logic for types that stops after the "head" (the top-level
structure) is constructed, which is enough for the self type-based impl
inlining analysis.