Improve into_slice_range inlining and add #[track_caller]

[saethlin]

View all comments

This is a solution to #157909. I think the root problem is that LLVM sees the many non-inlinable calls to slice_index_fail and decides that the function must be a bad inlining candidate. So I've shunted all the logic for reporting the right error into a single cold function and improved try_into_slice_range so that we only have to write all these checks once.

[saethlin]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 6a83d7f (6a83d7f17c32155a5e582df6cf5dfcfa8f6d1b67, parent: 01dfd79246f1b2d5f146616deff08223a840a9ae)

[rust-timer]

Finished benchmarking commit (6a83d7f): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.7%, secondary -2.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.7%	[1.3%, 4.1%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	2.7%	[1.3%, 4.1%]	2

Cycles

Results (primary 2.4%, secondary -15.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.4%	[2.4%, 2.4%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-15.3%	[-28.6%, -2.0%]	2
All ❌✅ (primary)	2.4%	[2.4%, 2.4%]	1

Binary size

Results (primary 0.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.1%, 0.9%]	4
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[-0.1%, 0.9%]	5

Bootstrap: 519.231s -> 519.196s (-0.01%)
Artifact size: 401.41 MiB -> 401.45 MiB (0.01%)

[saethlin]

@bors try @rust-timer queue

[rustbot]

r? @dingxiangfei2009

rustbot has assigned @dingxiangfei2009.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 20 candidates

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3550044 (35500440c17af6d90210ef58e386604fd5610cd5)
Base parent: 1162ebd (1162ebd81da49955aa94ef240cf7bb0bafad3f6a)

[rust-timer]

Finished benchmarking commit (3550044): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 1.7%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[1.7%, 1.7%]	1

Cycles

Results (secondary -4.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.3%	[-5.7%, -2.8%]	2
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.2%, secondary 0.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.1%, 0.5%]	4
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.1%, 0.5%]	4

Bootstrap: 521.352s -> 522.424s (0.21%)
Artifact size: 401.86 MiB -> 401.41 MiB (-0.11%)

[dingxiangfei2009]

r=me

Sorry for the delay, for I had to grab some context.

What do you think about the caller tracking on the other impls?

[rust-bors]

Unknown command "d+". Run @bors help or go to https://bors.rust-lang.org/help to see available commands.

[dingxiangfei2009]

@bors delegate+

😓

[rust-bors]

✌️ @saethlin, you can now approve this pull request!

If @dingxiangfei2009 told you to "r=me" after making some further change, then please make that change and post @bors r=dingxiangfei2009.

View changes since this delegation.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[saethlin]

@bors r=dingxiangfei2009

[rust-bors]

📌 Commit 81e7f00 has been approved by dingxiangfei2009

It is now in the queue for this repository.