Skip to content

Fold predicate fast path in canonicalizer and eager resolver #141442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 26, 2025

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented May 23, 2025

See individual commits.

r? lcnr

@rustbot rustbot added 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 May 23, 2025
@compiler-errors
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 23, 2025
bors added a commit that referenced this pull request May 23, 2025
Fold predicate fast path in canonicalizer and eager resolver

r? lcnr
@bors
Copy link
Collaborator

bors commented May 23, 2025

⌛ Trying commit e348ad2 with merge daa40a5...

@compiler-errors compiler-errors force-pushed the fast-path-pred branch 2 times, most recently from 951526f to 9ef3953 Compare May 23, 2025 13:25
@compiler-errors
Copy link
Member Author

@bors try

bors added a commit that referenced this pull request May 23, 2025
Fold predicate fast path in canonicalizer and eager resolver

r? lcnr
@bors
Copy link
Collaborator

bors commented May 23, 2025

⌛ Trying commit 9ef3953 with merge 6794d8c...

@compiler-errors
Copy link
Member Author

ok final time i promise, sorry bors

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

bors added a commit that referenced this pull request May 23, 2025
Fold predicate fast path in canonicalizer and eager resolver

r? lcnr
@bors
Copy link
Collaborator

bors commented May 23, 2025

⌛ Trying commit f96b8d5 with merge ed70264...

@bors
Copy link
Collaborator

bors commented May 23, 2025

☀️ Try build successful - checks-actions
Build commit: ed70264 (ed70264c6bb3cfdda1a0288aeaabcde9e1aea37f)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ed70264): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-5.7% [-15.7%, -0.1%] 16
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -1.5%, secondary -2.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.9% [2.9%, 2.9%] 1
Improvements ✅
(primary)
-1.5% [-1.5%, -1.5%] 1
Improvements ✅
(secondary)
-4.6% [-6.8%, -2.5%] 2
All ❌✅ (primary) -1.5% [-1.5%, -1.5%] 1

Cycles

Results (secondary -5.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-5.9% [-11.4%, -1.8%] 10
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 777.568s -> 776.075s (-0.19%)
Artifact size: 365.56 MiB -> 365.59 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 23, 2025
@Kobzol
Copy link
Contributor

Kobzol commented May 23, 2025

image

Great results! Only ~10x diff on nalgebra now, gogogo! 😆

@compiler-errors compiler-errors marked this pull request as ready for review May 24, 2025 17:25
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 24, 2025
@bors
Copy link
Collaborator

bors commented May 24, 2025

☔ The latest upstream changes (presumably #141506) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 26, 2025
@compiler-errors
Copy link
Member Author

compiler-errors commented May 26, 2025

@/bors treeclosed=10 (edited to prevent future pickup during sync)
@bors p=10

@compiler-errors
Copy link
Member Author

(CI issues it seems)

bors added a commit that referenced this pull request May 26, 2025
Fold predicate fast path in canonicalizer and eager resolver

See individual commits.

r? lcnr
@bors
Copy link
Collaborator

bors commented May 26, 2025

⌛ Testing commit ade2435 with merge 070bcfc...

@compiler-errors
Copy link
Member Author

@bors p=9 retry

@compiler-errors
Copy link
Member Author

@bors p=0

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
Image input checksum 3711b88b1827324ae8d9427cc0069e3335cb839f385c7f0dee4e0f5553e44567b4e82a97a5f4e2874946211296ffd47e2671327064f5333830f9e2a7c13abe68
##[group]Building docker image for dist-i686-linux
Docker version 28.0.4, build b8034c0
Error response from daemon: Get "https://ghcr.io/v2/": Get "https://ghcr.io/token?account=rust-lang&client_id=docker&offline_token=true&service=ghcr.io": net/http: request canceled (Client.Timeout exceeded while awaiting headers) (Client.Timeout exceeded while awaiting headers)
##[error]Process completed with exit code 1.
Post job cleanup.

@Kobzol
Copy link
Contributor

Kobzol commented May 26, 2025

GitHub is currently borked :( https://www.githubstatus.com/

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
Image input checksum af9f5e48d80befc25dc70c04d5246097cd4bfc6df4ab3b0cbe75b2e77ad98a0214e32db6526cf0487e06d5fd3576da10e4d9c5ae3543ae4494de13bb237e75cb
##[group]Building docker image for test-various
Docker version 28.0.4, build b8034c0
Error response from daemon: Get "https://ghcr.io/v2/": Get "https://ghcr.io/token?account=rust-lang&client_id=docker&offline_token=true&service=ghcr.io": net/http: request canceled (Client.Timeout exceeded while awaiting headers) (Client.Timeout exceeded while awaiting headers)
##[error]Process completed with exit code 1.
Post job cleanup.

@fee1-dead
Copy link
Member

fee1-dead commented May 26, 2025

@/bors treeclosed- (edited to prevent future pickup during sync)

Seems to be resolved.

@bors
Copy link
Collaborator

bors commented May 26, 2025

⌛ Testing commit ade2435 with merge 9c0bcb5...

@bors
Copy link
Collaborator

bors commented May 26, 2025

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 9c0bcb5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 26, 2025
@bors bors merged commit 9c0bcb5 into rust-lang:master May 26, 2025
7 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 26, 2025
Copy link
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 b5eb989 (parent) -> 9c0bcb5 (this PR)

Test differences

No test diffs found

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 9c0bcb514f49cd1e6a30affb2fe4cfca060129a2 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. aarch64-apple: 7237.0s -> 4627.1s (-36.1%)
  2. aarch64-gnu-debug: 3950.1s -> 4901.5s (24.1%)
  3. dist-aarch64-apple: 5166.3s -> 6152.9s (19.1%)
  4. x86_64-apple-2: 4591.8s -> 5442.5s (18.5%)
  5. dist-x86_64-apple: 9773.2s -> 11397.7s (16.6%)
  6. x86_64-apple-1: 8466.3s -> 7165.6s (-15.4%)
  7. dist-apple-various: 5486.3s -> 5912.7s (7.8%)
  8. x86_64-gnu-aux: 6163.2s -> 5841.6s (-5.2%)
  9. dist-x86_64-msvc-alt: 7515.2s -> 7797.8s (3.8%)
  10. x86_64-msvc-2: 6962.4s -> 6716.0s (-3.5%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9c0bcb5): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-6.1% [-15.7%, -0.2%] 15
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -0.0%, secondary -2.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.4% [1.3%, 1.5%] 2
Regressions ❌
(secondary)
3.3% [3.3%, 3.3%] 1
Improvements ✅
(primary)
-2.9% [-2.9%, -2.9%] 1
Improvements ✅
(secondary)
-5.2% [-6.8%, -3.6%] 2
All ❌✅ (primary) -0.0% [-2.9%, 1.5%] 3

Cycles

Results (secondary -6.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-6.3% [-11.9%, -2.3%] 12
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 775.728s -> 777.056s (0.17%)
Artifact size: 366.25 MiB -> 366.33 MiB (0.02%)

bors added a commit that referenced this pull request May 26, 2025
add additional `TypeFlags` fast paths

Some crates, e.g. `diesel`, have items with a lot of where-clauses (more than 150). In these cases checking the `TypeFlags` of the whole `param_env` can be very beneficial.

This adds `fn fold_clauses` to mirror the existing `fn visit_clauses` and then uses this in folders which fold `ParamEnv`s.

Split out from #141451, depends on #141442.

r? `@compiler-errors`
bors added a commit that referenced this pull request May 29, 2025
add additional `TypeFlags` fast paths

Some crates, e.g. `diesel`, have items with a lot of where-clauses (more than 150). In these cases checking the `TypeFlags` of the whole `param_env` can be very beneficial.

This adds `fn fold_clauses` to mirror the existing `fn visit_clauses` and then uses this in folders which fold `ParamEnv`s.

Split out from #141451, depends on #141442.

r? `@compiler-errors`
RalfJung pushed a commit to RalfJung/miri that referenced this pull request May 29, 2025
add additional `TypeFlags` fast paths

Some crates, e.g. `diesel`, have items with a lot of where-clauses (more than 150). In these cases checking the `TypeFlags` of the whole `param_env` can be very beneficial.

This adds `fn fold_clauses` to mirror the existing `fn visit_clauses` and then uses this in folders which fold `ParamEnv`s.

Split out from rust-lang/rust#141451, depends on rust-lang/rust#141442.

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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
Development

Successfully merging this pull request may close these issues.

8 participants