Repository navigation
Rollup of 5 perf-sensitive pull requests - #165107
Merged
Merged
Conversation
…large structs This improves compile time for crates that derive Debug for large structs (e.g., ripgrep). It also reduces metadata size for some of those.
This avoids one more string and makes the derive code slightly nicer as we don't have to track whether we're on last item
We do this by changing the variable to a closure and only call it when it's needed
In `instantiate_and_apply_query_response`, instantiate the two instantiatable parts of the response separately. This requires adding a new method `instantiate_value` to `SolverDelegate` (r-a will need updating separately) and making a function of the same name in `rustc_infer` public. Do a similar thing for `State` in `instantiate_canonical_state`. This is equivalent behaviour that will enable further changes in subsequent commits.
Both `unify_query_var_values` call sites are preceded by a call to `instantiate_value` on the `var_values`. If instantiation changes any of the elements we end up interning an entirely new list, through `fold_list`. This commit changes things so that `unify_query_var_values` does instantiation on individual elements as it iterates, and avoids having to intern an entire list. This changes the caching behaviour in some cases, but overall it's a perf win.
This avoids a lot of calls to `instantiate_value`.
`Response` has `is_identity` and `is_identity_modulo_regions` methods. These are reasonably hot and traverse the response's `var_values` in very similar ways. This commit merges these two methods into a single method that returns a new three-value enum `IsIdentity`. It also pre-computes that `IsIdentity` for each response and stores it in the new `Response::var_values_is_identity` field. This is a perf win because it avoids repeated traversals.
If the response is an identity, and there are no external constraints, and the response var kinds match the canonical goal var kinds, this function is a no-op.
It provides a more principled and type-safe way to extract and process parts of a `Canonical` value.
They are like `TokenStream` and `TokenTree`, but with the additional `AttrTokenTree::AttrsTarget` variant that stores the attributes and tokens for an attribute target. This duplication is annoying, and also leads to some inefficiencies when we have to produce an `AttrTokenStream` only to then convert it to a `TokenStream`. The new `LazyAttrTokenStreamInner::to_token_stream_with` takes a `configure` closure. This closure is passed through various layers down to `make_token_trees`, where it controls cfg expansion. Callers that don't do expansion pass a trivial `keep_all` closure. `try_configure_tokens` passes a non-trivial `configure` closure. Overall this change makes the code shorter and slightly improves compile times. One minor behavioural change on an error case: if parse recovery finishes with an unmatched opening delimiter, `make_token_trees` drops the caller's preceding trees; the old code kept them. This is obscure and no existing tests hit it. The next commit will rename `LazyAttrTokenStream` to `LazyTokenStream`. It's a separate commit because it is uninteresting churn.
By removing the `Attr`, which is no longer necessary.
In `check_expr_lit`, if we have an integer literal and the expected type
is an integer type var, we currently create a new type var that is later
unified with the expected one. This commit changes it to just reuse the
existing expected one. This avoids lots of unnecessary integer type var
creation and unification. (And likewise for float type vars.)
This is a big win on `deep-vector`, but it doesn't help with other
benchmarks, unfortunately.
Only one ui test is changed, for the better:
- Previously it printed `{integer}` because there were two distinct
integer variables. Now there is one and the types-are-equal error
message elision occurs and it is shown as `_`. (Same as what you'd see
if the call was `test(&mut 7i32, &7i32)` with known types.)
- There is now also a useful `help` subdiagnostic.
Given how simple the change is, I think it's worthwhile.
New solver opts for `diesel` (part 2) A sequel to rust-lang#163981. Details in individual commits. Local measurements: ``` solver icounts (B) ------ ----------- old 37.9 new (orig) 49.6 (1.31x) new (this PR) 45.4 (1.20x) ``` Also some small improvements on a range of other benchmarks. r? @lcnr
…ethercote perf: emit field names as a concatenated string in derive(Debug) for structs Similar idea as rust-lang#162535 and previous attempts. Again, I also noticed that no test fails when I mess up the string, so I added a test case.
Avoid stdlib's `impl Hash for Discriminant` for stable hashing Of all the different ways we currently leak compiler/stdlib details into stable hashes, relying on `Discriminant::hash` seems high on the list of ones that could cause actual problems in the future. This PR therefore removes `impl StableHash for Discriminant`, and replaces it with an explicit negative impl as a roadblock to anyone who might unknowingly try to reintroduce the problem. To make this work, `#[derive(StableHash)]` now invents its own fake discriminant based on the syntactic ordering of enum variants. That should keep the hash values consistent across compiler stages. In places where we were hashing Discriminant manually, this PR introduces explicit “tag” enums as seen in rust-lang#164458. --- This change necessarily perturbs the hashes included in legacy-symbol-mangling symbols, as demonstrated in the re-blessed tests. This should be fine, as I believe the exact hash values are considered unstable anyway and have changed in the past.
…type-vars, r=BoxyUwU
Avoid unnecessary numeric type vars
In `check_expr_lit`, if we have an integer literal and the expected type is an integer type var, we currently create a new type var that is later unified with the expected one. This commit changes it to just reuse the existing expected one. This avoids lots of unnecessary integer type var creation and unification. (And likewise for float type vars.)
This is a big win on `deep-vector`, but it doesn't help with other benchmarks, unfortunately.
Only one ui test is changed, for the better:
- Previously it printed `{integer}` because there were two distinct integer variables. Now there is one and the types-are-equal error message elision occurs and it is shown as `_`. (Same as what you'd see if the call was `test(&mut 7i32, &7i32)` with known types.)
- There is now also a useful `help` subdiagnostic.
Given how simple the change is, I think it's worthwhile.
r? @BoxyUwU
…petrochenkov Remove `AttrTokenStream` and `AttrTokenTree` These are very similar to `TokenStream` and `TokenTree`, which has annoyed me for a long time. This PR removes them, making the code simpler and a tiny bit faster. Details in individual commits. r? @petrochenkov
rustbot
added
S-waiting-on-review
Member
Author
|
@bors r+ p=1 force |
Contributor
rust-bors
Bot
added
S-waiting-on-bors
JonathanBrouwer
changed the title
This comment has been minimized.
This comment has been minimized.
rust-bors Bot
pushed a commit
that referenced
this pull request
Oct 29, 2026
…uwer Rollup of 5 perf-sensitive pull requests Successful merges: - #164234 (New solver opts for `diesel` (part 2)) - #164726 (perf: emit field names as a concatenated string in derive(Debug) for structs) - #164953 (Avoid stdlib's `impl Hash for Discriminant` for stable hashing) - #164965 (Avoid unnecessary numeric type vars) - #165022 (Remove `AttrTokenStream` and `AttrTokenTree`)
Member
Author
|
@bors yield |
Contributor
|
Auto build was cancelled. Cancelled workflows: The next pull request likely to be tested is #165095. |
This comment has been minimized.
This comment has been minimized.
rust-bors
Bot
added
merged-by-bors
Contributor
This was referenced Oct 29, 2026
Contributor
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 ce37c4f (parent) -> 96a18bf (this PR) Test differencesShow 34 test diffs34 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 96a18bf5ffa2b24f9cf3d1c731a9eb121c1acad9 --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 |
Contributor
|
📌 Perf builds for each rolled up PR:
parent commit: ce37c4ff83 In the case of a perf regression, run the following command with the SHAs of each PR you suspect might be the cause: |
Collaborator
|
Finished benchmarking commit (96a18bf): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our benchmarks found a performance regression caused by this PR. Next Steps:
@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 0.0%, secondary -5.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.5%, secondary -5.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.7%, secondary 0.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 490.291s -> 487.353s (-0.60%) |
Contributor
|
@rust-timer triage all |
Collaborator
Running triage with 49 benchmarksTriage only executes the benchmarks on rollup members, that were changed significantly on the rollup.
#164234 b09da0c New solver opts for
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Successful merges:
diesel(part 2) #164234 (New solver opts fordiesel(part 2))impl Hash for Discriminantfor stable hashing #164953 (Avoid stdlib'simpl Hash for Discriminantfor stable hashing)AttrTokenStreamandAttrTokenTree#165022 (RemoveAttrTokenStreamandAttrTokenTree)r? @ghost
Create a similar rollup