Shallowly bail from coerce_unsized more#142941
Conversation
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
[perf] Shallowly bail from `coerce_unsized` more
|
TODO: test with a fast path for |
This comment has been minimized.
This comment has been minimized.
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (6a35eeb): comparison URL. Overall result: ✅ improvements - 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 countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -1.2%, secondary -5.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -4.5%, secondary -15.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary -0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 688.989s -> 689.016s (0.00%) |
|
holy fuck lol |
e0b68fe to
3d115ac
Compare
|
@bors2 try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
[perf] Shallowly bail from `coerce_unsized` more
|
Anyways this is ready to review. r? @lcnr |
|
rustbot has assigned @petrochenkov. Use |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Crashes should be fixed by #142976; they're only coincidentally fixed here by the fast path bc we don't end up trying |
coerce_unsized morecoerce_unsized more
|
Finished benchmarking commit (1eb1a24): comparison URL. Overall result: ✅ improvements - 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 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 -3.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.0%, secondary -12.2%)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: 689.383s -> 687.827s (-0.23%) |
| #[instrument(skip(self), level = "debug")] | ||
| fn coerce_unsized(&self, mut source: Ty<'tcx>, mut target: Ty<'tcx>) -> CoerceResult<'tcx> { | ||
| source = self.shallow_resolve(source); | ||
| target = self.shallow_resolve(target); |
There was a problem hiding this comment.
why remove these, are we guaranteed that they are shallow resolved? if so, please convert them into debug asserts
There was a problem hiding this comment.
yes, we shallowly resolve at the top of the coerce entrypoint
| | ty::Infer(ty::IntVar(_) | ty::FloatVar(_)) | ||
| | ty::Str | ||
| | ty::Array(_, _) | ||
| | ty::Slice(_) |
There was a problem hiding this comment.
array to slice is always Deref an not CoerceUnsized? 🤔
There was a problem hiding this comment.
Slices are the RHS for Unsize, not CoerceUnsized. The latter is for pointers not data.
There was a problem hiding this comment.
ah yeah, remember how this works again/looked it up
|
r=me after adding the debug asserts |
3d115ac to
0d37587
Compare
|
Added a bunch of debug assertions @bors r=lcnr |
Shallowly bail from `coerce_unsized` more We do a *lot* of coercion in HIR typeck. Most of the time we're just coercing a type to itself, but we always try `coerce_unsized` even if it's not necessary. Let's avoid doing that by adding a fast path to `coerce_unsized`; see the comment in that function.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
|
spurious? @bors retry |
|
☀️ 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 35f6036 (parent) -> f191420 (this PR) Test differencesShow 2 test diffs2 doctest diffs were found. These are ignored, as they are noisy. Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard f19142044f270760ce0ebc03b2c3a44217d8703d --output-dir test-dashboardAnd 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 (f191420): comparison URL. Overall result: ✅ improvements - no action needed@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 (secondary -3.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary -2.0%, secondary -12.4%)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: 695.714s -> 695.172s (-0.08%) |
We do a lot of coercion in HIR typeck. Most of the time we're just coercing a type to itself, but we always try
coerce_unsizedeven if it's not necessary.Let's avoid doing that by adding a fast path to
coerce_unsized; see the comment in that function.