Type fallback refactorings - #159026
Conversation
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Type fallback refactorings
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (6a25a06): 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 @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 (primary 3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -5.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 487.967s -> 488.861s (0.18%) |
68f06e3 to
8992f0e
Compare
|
I think the perf regression is noise. |
| .map(|&ty_id| self.shallow_resolve(Ty::new_var(self.tcx, ty_id))) | ||
| .filter_map(|ty| ty.ty_vid()) | ||
| .map(|vid| self.root_var(vid)) |
There was a problem hiding this comment.
that should also be a probe_with_root_vid i think? seems like a very cumbersome way to get the root vid of a ty var
There was a problem hiding this comment.
The problem is that everything to do with type variables is private. See 680fc04 for what needs to be made public.
|
unsure about the last commit after applying my other review comments, but r=me after nits on the other changes |
680fc04 to
4ac3cec
Compare
|
@bors r=lcnr (see #159026 (comment); I've also applied suggestion from #159026 (comment)) |
This comment has been minimized.
This comment has been minimized.
Type fallback refactorings - Don't merge different kinds of infer variables into `Ty` before matching them back - Do fallback only on root vars (this is semantically equivalent to what we currently do; unifying any of the infer vars from the same equivalence class necesserily affects all of them) (this makes #159003 a bit less hacky though) - Do some simplification that has been allowed by the previous changes cc @lcnr
|
💔 Test for 9495eda failed: CI. Failed job:
|
|
@bors retry |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
This comment has been minimized.
This comment has been minimized.
|
The job Click to see the possible cause of the failure (guessed by this bot)Important For more information how to resolve CI failures of this job, visit this link. |
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
|
Finished benchmarking commit (22950ab): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Our 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 -0.9%, secondary 0.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.1%, secondary -0.9%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary -0.0%, secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 457.809s -> 454.551s (-0.71%) |
View all comments
r? types
Tybefore matching them backcc @lcnr