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.

Sign up for GitHub

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

Jump to bottom

some inspect improvements #115751

Merged
merged 5 commits into from
Sep 14, 2023
Merged

some inspect improvements #115751

merged 5 commits into from
Sep 14, 2023

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Sep 11, 2023

split from #114810 because I still want to experiment a bunch with that PR and these changes are self-contained.

r? @compiler-errors

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Sep 11, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 11, 2023

Some changes occurred to the core trait solver

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

@lcnr lcnr changed the title Inspect cleanup some inspect improvements Sep 11, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
lcnr added 3 commits September 11, 2023 11:51
@lcnr
split GoalEvaluation and CanonicalGoalEvaluation
fc452e2 
the unnormalized goal is in the callers inference context, while
anything inside of the `CanonicalGoalEvaluation` is inside of
a new one.
@lcnr
inspect: handle None in nested
01f3da6
@lcnr
revision -> iteration for added_goals_evaluation
006d599
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lcnr
dedup GoalEvaluationStep and GoalCandidate
eac55ee 
also handle 2 panics when dumping proof trees for the whole test suite

- need to actually tell the proof tree builder about overflow
- need to handle a recursion_limit of 0 :<
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 13, 2023

📌 Commit eac55ee has been approved by compiler-errors

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 Sep 13, 2023
@bors
Copy link
Contributor

bors commented Sep 14, 2023

⌛ Testing commit eac55ee with merge 2394959...

@bors
Copy link
Contributor

bors commented Sep 14, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 2394959 to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Sep 14, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 2394959 to master...

@bors bors added merged-by-bors This PR was explicitly merged by bors. labels Sep 14, 2023
@bors bors merged commit 2394959 into rust-lang:master Sep 14, 2023
@rustbot rustbot added this to the 1.74.0 milestone Sep 14, 2023
@bors bors mentioned this pull request Sep 14, 2023
Draft
@lcnr lcnr deleted the inspect-cleanup branch September 14, 2023 06:57
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2394959): 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)		 1.0% [0.6%, 1.3%] 7
Regressions ❌
(secondary)		 0.6% [0.6%, 0.6%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 1.0% [0.6%, 1.3%] 7

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.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)		 3.7% [3.7%, 3.7%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.6% [0.6%, 0.6%] 1

Cycles

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)		 3.9% [2.1%, 5.5%] 4
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 3.9% [2.1%, 5.5%] 4

Binary size

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

Bootstrap: 631.545s -> 632.761s (0.19%)
Artifact size: 317.98 MiB -> 317.92 MiB (-0.02%)

@rustbot rustbot added the perf-regression Performance regression. label Sep 14, 2023
@lcnr lcnr mentioned this pull request Sep 14, 2023
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 18, 2023
@Dylan-DPC
Rollup merge of rust-lang#115838 - lcnr:added-goals, r=compiler-errors
bcfe87d 
inspect: closer to proof trees for coherence

a continuation of rust-lang#115751. Now explicitly store the added goals

r? `@compiler-errors`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 18, 2023
@matthiaskrgr
Rollup merge of rust-lang#115838 - lcnr:added-goals, r=compiler-errors
2d170f1 
inspect: closer to proof trees for coherence

a continuation of rust-lang#115751. Now explicitly store the added goals

r? ``@compiler-errors``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 18, 2023
@matthiaskrgr
Rollup merge of rust-lang#115838 - lcnr:added-goals, r=compiler-errors
944c413 
inspect: closer to proof trees for coherence

a continuation of rust-lang#115751. Now explicitly store the added goals

r? ```@compiler-errors```
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 18, 2023
@rust-timer
Unrolled build for rust-lang#115838
2a5c7ed 
Rollup merge of rust-lang#115838 - lcnr:added-goals, r=compiler-errors

inspect: closer to proof trees for coherence

a continuation of rust-lang#115751. Now explicitly store the added goals

r? ```@compiler-errors```
@Mark-Simulacrum Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Sep 19, 2023
@Mark-Simulacrum
Copy link
Member

This looks like infrequent noise to me, so marking as triaged.

image

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

@compiler-errors compiler-errors

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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Milestone
1.74.0
Development

Successfully merging this pull request may close these issues.
7 participants
@lcnr @rustbot @rust-log-analyzer @compiler-errors @bors @rust-timer @Mark-Simulacrum