Skip to content
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

Branch Clause from Predicate #104846

Merged

Conversation

spastorino
Copy link
Member

r? @oli-obk

This is part of what's proposed in rust-lang/compiler-team#531

@rustbot rustbot added 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 24, 2022
@rustbot
Copy link
Collaborator

rustbot commented Nov 24, 2022

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@spastorino spastorino force-pushed the santa-clauses-make-goals-early-christmas- branch from 7387f19 to 4f544d0 Compare November 24, 2022 21:19
@rust-log-analyzer

This comment has been minimized.

@spastorino spastorino force-pushed the santa-clauses-make-goals-early-christmas- branch from 4f544d0 to 5be8469 Compare November 25, 2022 02:05
@rust-log-analyzer

This comment has been minimized.

@spastorino spastorino force-pushed the santa-clauses-make-goals-early-christmas- branch from 5be8469 to 10f501a Compare November 25, 2022 02:31
@rust-log-analyzer

This comment has been minimized.

@spastorino spastorino force-pushed the santa-clauses-make-goals-early-christmas- branch from 10f501a to 79281ed Compare November 25, 2022 02:51
@spastorino spastorino force-pushed the santa-clauses-make-goals-early-christmas- branch from 79281ed to 974e283 Compare November 25, 2022 03:05
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
.......... (80/90)
.......... (90/90)


/checkout/src/test/rustdoc-gui/basic-code.goml basic-code... FAILED
[ERROR] (line 3) Error: Execution context was destroyed, most likely because of a navigation.: for command `assert-count: (".src-line-numbers", 1)`
Build completed unsuccessfully in 0:02:22

@oli-obk
Copy link
Contributor

oli-obk commented Nov 25, 2022

@bors r+ p=1 bitrotty

@bors
Copy link
Contributor

bors commented Nov 25, 2022

📌 Commit 974e283 has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 25, 2022
@bors
Copy link
Contributor

bors commented Nov 25, 2022

⌛ Testing commit 974e283 with merge 051cab2...

@bors
Copy link
Contributor

bors commented Nov 25, 2022

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 051cab2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 25, 2022
@bors bors merged commit 051cab2 into rust-lang:master Nov 25, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 25, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (051cab2): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.8% [0.2%, 2.4%] 38
Regressions ❌
(secondary)
0.4% [0.3%, 0.8%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.8% [0.2%, 2.4%] 38

Max RSS (memory usage)

Results

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.1% [1.8%, 2.3%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.0% [-4.7%, -2.9%] 3
All ❌✅ (primary) - - 0

Cycles

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

@rustbot rustbot added the perf-regression Performance regression. label Nov 25, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Nov 28, 2022

The diesel regression is

  244,159,238  ???:<rustc_trait_selection::traits::fulfill::FulfillmentContext as rustc_infer::traits::engine::TraitEngine>::register_predicate_obligation
  128,790,199  ???:rustc_trait_selection::traits::wf::predicate_obligations
 -112,762,937  ???:<&mut rustc_hir_analysis::check::wfcheck::check_where_clauses::{closure
   70,951,310  ???:rustc_middle::ty::util::fold_list::<rustc_infer::infer::canonical::canonicalizer::Canonicalizer, rustc_middle::ty::Predicate, <&rustc_middle::ty::list::List<rustc_middle::ty::Predicate> as rustc_middle::ty::fold::TypeFoldable>::try_fold_with<rustc_infer::infer::canonical::canonicalizer::Canonicalizer>::{closure
   37,831,490  ???:<rustc_middle::ty::context::CtxtInterners>::intern_predicate
   24,179,850  ???:<rustc_infer::infer::InferCtxt>::resolve_vars_if_possible::<rustc_infer::traits::Obligation<rustc_middle::ty::Predicate>>
   21,434,192  ???:<rustc_trait_selection::traits::fulfill::FulfillProcessor as rustc_data_structures::obligation_forest::ObligationProcessor>::process_obligation

We should figure out what part of this PR caused this

@spastorino
Copy link
Member Author

@rustbot label +perf-regression-triaged

We're aware of this regression, and I've opened #105060 to track it. We would need to intern Clauses in order to make the memory used lower, comparisons (comparing references) faster and such. After @oli-obk lands some stuff he is doing, I'm going complete the thing by interning Clauses.

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Nov 30, 2022
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 1, 2022
…early-christmas-🎄, r=oli-obk

Branch Clause from Predicate

r? `@oli-obk`

This is part of what's proposed in rust-lang/compiler-team#531
@oli-obk oli-obk deleted the santa-clauses-make-goals-early-christmas-🎄 branch January 27, 2023 08:24
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. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants