Rollup of 7 pull requests #88881
Rollup of 7 pull requests #88881
Conversation
backport-of: none
This expands the API to be more flexible, allowing for more visitation patterns on graphs. This will be useful to avoid extra datasets (and allocations) in cases where the expanded DFS API is sufficient. This also fixes a bug with the previous DFS constructor, which left the start node not marked as visited (even though it was immediately returned).
They can be obtained by accessing the `TyCtxt` where they are needed.
Local variables can never be exported.
The order of the `where` bounds on auto trait impls changed because rustdoc currently sorts auto trait `where` bounds based on the `Debug` output for the bound. Now that the bounds have an actual `Res`, they are being unintentionally sorted by their `DefId` rather than their path. So, I had to update a test for the change in ordering of the rendered bounds.
If the path is for a trait, it is always true that `trait_did == Some(did)`, so instead, `external_path()` now takes an `is_trait` boolean.
…estebank Detect stricter constraints on gats where clauses in impls vs trait I might try to see if I can do a bit more to improve these diagnostics, but any initial feedback is appreciated. I can also do any additional work in a followup PR. r? `@estebank`
rustc: Remove local variable IDs from `Export`s Local variables can never be exported.
…, r=pietroalbini Remove extra unshallow from cherry-pick checker This is already done by https://github.com/rust-lang/rust/blob/13db8440bbbe42870bc828d4ec3e965b38670277/src/ci/init_repo.sh#L32-L36 on the beta channel, and git throws an error if you attempt to unshallow an already non-shallow repository. r? ```@pietroalbini```
generic_const_exprs: use thir for abstract consts instead of mir Changes `AbstractConst` building to use `thir` instead of `mir` so that there's less chance of consts unifying when they shouldn't because lowering to mir dropped information (see `abstract-consts-as-cast-5.rs` test) r? `@lcnr`
…h726 Rework DepthFirstSearch API This expands the API to be more flexible, allowing for more visitation patterns on graphs. This will be useful to avoid extra datasets (and allocations) in cases where the expanded DFS API is sufficient. This also fixes a bug with the previous DFS constructor, which left the start node not marked as visited (even though it was immediately returned). Commit written by ```@nikomatsakis``` originally, cherry picked from several commits in work on never type stabilization, but stands alone.
rustdoc: Cleanup `clean` part 1 Split out from rust-lang#88379. These commits are completely independent of each other, and each is a fairly small change (the last few are new commits; they are not from rust-lang#88379): - Remove unnecessary `Cache.*_did` fields - rustdoc: Get symbol for `TyParam` directly - Create a valid `Res` in `external_path()` - Remove unused `hir_id` parameter from `resolve_type` - Fix redundant arguments in `external_path()` - Remove unnecessary `is_trait` argument - rustdoc: Cleanup a pattern match in `external_generic_args()` r? ``@jyn514``
explicitly link to external `ena` docs we currently do not link to the docs of `ena`: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_infer/infer/struct.InferCtxtInner.html#method.const_unification_table
|
@bors r+ p=1 |
|
📌 Commit 146aee6 has been approved by |
bors
added
the
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (c7dbe7a): comparison url. Summary: This change led to large relevant regressions 😿 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
|
It feels like #88709 is the only likely culprit but it's also feature gated. Unsure what's going on here |
|
Focusing in on the top regression, inflate check full, cachegrind shows the regression centering on process_obligations: From there, we want to check if the function is called more often or each call became more expensive. For this, callgrind is useful. Rerun the diff_local with callgrind: Then, the default view doesn't show call counts, but you can pass We look at the numbers in (...) after the callers. In this case, they're the same, so each function got a little more expensive -- with thousands of calls, even a slight difference in instruction counts can blow up to a big change. No direct changes were made in the source code though. There is an average of 2,840 more instructions per call (inclusive of functions called by process obligations), but it's not really clear what the cause is. A cursory inspection suggests that the new code has a larger stack allocation (up by 32 bytes), so presumably there's more load/spills... but the reason for the larger stack is unclear. It seems plausible that this is due to shifts in optimization choices made through PGO that in this case were "bad". Inflate is one of the benchmarks run in CI for PGO, but it's still possible for that to produce negative results. Reading the assembly to figure out what is causing the loads and spills without debuginfo doesn't seem worthwhile to invest into right now (and reproducing the build with debuginfo is probably hard). My guess is that shifts elsewhere in the program slightly shifted the profile for this function or something along those lines and that caused this shift. It seems quite possible that no particular PR is actually fully responsible for this change. |
|
FWIW while looking into this I also filed #89495 which may be of some help, but it's not a mitigation for this regression, just a sideline optimization. |
Mark-Simulacrum
added
the
perf-regression-triaged
Successful merges:
Exports #88677 (rustc: Remove local variable IDs fromExports)cleanpart 1 #88810 (rustdoc: Cleanupcleanpart 1)enadocs #88813 (explicitly link to externalenadocs)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup