Skip to content

New solver opts for diesel (part 2) - #164234

Open
nnethercote wants to merge 6 commits into
rust-lang:mainfrom
nnethercote:diesel-opts-2
Open

New solver opts for diesel (part 2)#164234
nnethercote wants to merge 6 commits into
rust-lang:mainfrom
nnethercote:diesel-opts-2

Conversation

@nnethercote

@nnethercote nnethercote commented Oct 13, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

A sequel to #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

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Oct 13, 2026
@nnethercote

nnethercote commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Sign in to view
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 13, 2026
rust-bors Bot pushed a commit that referenced this pull request Oct 13, 2026
@bors
Auto merge of #164234 - nnethercote:diesel-opts-2, r=<try>
f9f6136 
New solver opts for `diesel` (part 2)
@rust-bors

rust-bors Bot commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: f9f6136 (f9f6136becce4c35d1a8b5f9a96bd15e9b06a9de)
Base parent: 88d9824 (88d982422ac482951195284e27f5bcb0d28f298b)

@rust-timer

This comment has been minimized.

Sign in to view
@rust-timer

rust-timer commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (f9f6136): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking 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.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 0.2% [0.1%, 0.4%] 2
Improvements ✅
(primary)		 -0.6% [-1.5%, -0.1%] 14
Improvements ✅
(secondary)		 -3.6% [-12.6%, -0.1%] 33
All ❌✅ (primary) -0.6% [-1.5%, -0.1%] 14

Max RSS (memory usage)

Results (secondary -2.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 5.0% [5.0%, 5.0%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -3.2% [-4.7%, -1.9%] 6
All ❌✅ (primary) - - 0

Cycles

Results (primary -3.5%, secondary -4.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 4.1% [2.9%, 5.4%] 2
Improvements ✅
(primary)		 -3.5% [-4.3%, -2.1%] 7
Improvements ✅
(secondary)		 -5.5% [-10.2%, -2.4%] 17
All ❌✅ (primary) -3.5% [-4.3%, -2.1%] 7

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 512.754s -> 487.269s (-4.97%)
Artifact size: 406.91 MiB -> 409.16 MiB (0.55%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Oct 13, 2026
@nnethercote

nnethercote commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Perf results look excellent, especially for diesel and typenum with the new solver.

LLM disclosure: some of the ideas in the PR came from an LLM. I wrote the code and text myself.

@nnethercote
nnethercote marked this pull request as ready for review October 13, 2026 04:25
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 13, 2026
@rustbot

rustbot commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 13, 2026
@nnethercote

nnethercote commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

@lcnr: the first four commits are straightforward. The last one is a bit trickier and deserves careful checking.

lcnr
lcnr reviewed Oct 13, 2026
View reviewed changes
Comment thread compiler/rustc_next_trait_solver/src/canonical/mod.rs Outdated
lcnr
lcnr reviewed Oct 13, 2026
View reviewed changes
Comment thread compiler/rustc_next_trait_solver/src/canonical/mod.rs Outdated
lcnr
lcnr reviewed Oct 13, 2026
View reviewed changes
Comment thread compiler/rustc_type_ir/src/solve/mod.rs Outdated
lcnr
lcnr reviewed Oct 13, 2026
View reviewed changes
Comment thread compiler/rustc_next_trait_solver/src/canonical/mod.rs
@nnethercote
Instantiate a response in pieces
19a9a08 
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.
@nnethercote
Optimize unify_query_var_values
fdbbf3f 
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.
@nnethercote
Special-case canonical bound vars in unify_query_var_values
75cca6d 
This avoids a lot of calls to `instantiate_value`.
@nnethercote
Introduce IsIdentity type
52d3937 
`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.
@nnethercote
Add an early return to instantiate_and_apply_query_response
211d8e5 
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.
@nnethercote
Introduce Canonicalized and other clarifications
1d54383 
It provides a more principled and type-safe way to extract and process
parts of a `Canonical` value.
@rustbot

rustbot commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@nnethercote

nnethercote commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

I have addressed the comments.

  • Renamed the is_identity field as var_values_is_identity.
  • Some more comments.
  • Added a Canonicalized type for projections.
  • Added an instantiate_generic_arg function.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lcnr lcnr lcnr left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@lcnr lcnr

Labels

perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nnethercote @rust-timer @rustbot @lcnr