-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Instantiate auto trait/Copy
/Clone
/Sized
before computing constituent types binder
#143538
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
Conversation
@bors2 try @rust-timer queue and needs crater too when that's done |
This comment has been minimized.
This comment has been minimized.
Instantiate auto trait before computing higher-ranked constituent types This will need an FCP and a crater run, but I think this makes the binder logic w.r.t. auto traits and coroutines a bit simpler. r? lcnr
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (ad95a52): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -2.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -4.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 460.528s -> 461.621s (0.24%) |
why does this need an FCP? we enter the merged binders right after calling We always instantiate the binder after calling |
41eb6d8
to
9cb805e
Compare
Yeah, I don't see why this has an FCP. Was just being a bit conservative, but there's no observable behavioral change here that I can see. |
@bors2 try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
Instantiate auto trait before computing higher-ranked constituent types This makes the binder logic w.r.t. auto traits and coroutines a bit simpler. r? lcnr
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (18fd892): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary 2.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -0.8%, secondary 1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 465.909s -> 464.854s (-0.23%) |
Copy
/Clone
/Sized
before computing constituent types binder
This is either noise or insignificant wrt the cleanup @rustbot ready |
☔ The latest upstream changes (presumably #143621) made this pull request unmergeable. Please resolve the merge conflicts. |
| ty::Str | ||
| ty::Slice(..) | ||
| ty::Foreign(..) | ||
| ty::Ref(_, _, hir::Mutability::Mut) => {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment is wrong for &mut _
🤔
r=me after rebase + nit |
9cb805e
to
bbb409c
Compare
@bors r=lcnr |
🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened. |
☀️ Test successful - checks-actions |
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 e3fccdd (parent) -> e43d139 (this PR) Test differencesShow 10 test diffs10 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard e43d139a82620a268d3828a73e12a8679339e8f8 --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (e43d139): comparison URL. Overall result: ❌ regressions - please read the text belowOur 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 7.5%, secondary -1.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 464.444s -> 464.828s (0.08%) |
This makes the binder logic w.r.t. coroutines a bit simpler.
r? lcnr