avoid phi node for pointers flowing into Vec appends #130998

the8472 · 2024-09-28T22:33:56Z

Elide temporary allocations in patterns like vec.append(slice.to_vec())

related discussion: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/nocapture.20and.20allocation.20elimination

the8472 · 2024-09-28T22:35:18Z

On its own I don't expect this to do much, we also need llvm/llvm-project#110280 to get memcpy propagation.

But lets see what the perf impact is without the LLVM changes.

@bors try @rust-timer queue

bors · 2024-09-28T22:36:28Z

⌛ Trying commit 9a03f37 with merge fa341e6...

avoid phi node for pointers flowing into Vec appends related discussion: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/nocapture.20and.20allocation.20elimination r? ghost

bors · 2024-09-29T00:21:26Z

☀️ Try build successful - checks-actions
Build commit: fa341e6 (fa341e6bb26c4367ebce0bf9e5583eb3df53c79f)

rust-timer · 2024-09-29T04:20:24Z

Finished benchmarking commit (fa341e6): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

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)	0.5%	[0.3%, 0.9%]	5
Regressions ❌ (secondary)	0.2%	[0.2%, 0.2%]	1
Improvements ✅ (primary)	-0.3%	[-0.4%, -0.3%]	2
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.2%]	1
All ❌✅ (primary)	0.3%	[-0.4%, 0.9%]	7

Max RSS (memory usage)

Results (primary -0.1%, secondary 0.5%)

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)	2.5%	[2.5%, 2.5%]	1
Regressions ❌ (secondary)	1.7%	[1.7%, 1.7%]	1
Improvements ✅ (primary)	-2.6%	[-2.6%, -2.6%]	1
Improvements ✅ (secondary)	-0.7%	[-0.7%, -0.7%]	1
All ❌✅ (primary)	-0.1%	[-2.6%, 2.5%]	2

Cycles

Results (secondary -0.3%)

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
Regressions ❌ (secondary)	2.4%	[2.4%, 2.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-3.1%, -3.1%]	1
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.0%, secondary 0.0%)

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.0%	[0.0%, 0.0%]	13
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	31
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-0.1%, 0.0%]	14

Bootstrap: 769.092s -> 770.401s (0.17%)
Artifact size: 341.40 MiB -> 341.45 MiB (0.01%)

the8472 · 2024-09-29T08:53:58Z

As expected it doesn't do much on its own, let's wait for the LLVM change.

the8472 · 2024-09-30T23:10:18Z

@bors try @rust-timer queue

bors · 2024-09-30T23:11:30Z

⌛ Trying commit c44e0f4 with merge fd59d69...

avoid phi node for pointers flowing into Vec appends related discussion: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/nocapture.20and.20allocation.20elimination r? ghost

bors · 2024-10-01T00:55:42Z

☀️ Try build successful - checks-actions
Build commit: fd59d69 (fd59d692ac24ed3d88de1b531716c0c053cc7680)

rust-timer · 2024-10-01T02:15:07Z

Finished benchmarking commit (fd59d69): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

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)	0.5%	[0.3%, 0.8%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.7%, -0.3%]	5
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.1%]	11
All ❌✅ (primary)	0.1%	[-0.7%, 0.8%]	10

Max RSS (memory usage)

Results (primary 2.5%)

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)	5.1%	[4.1%, 6.2%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.8%	[-2.8%, -2.8%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.5%	[-2.8%, 6.2%]	3

Cycles

Results (secondary -2.8%)

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

Binary size

Results (primary 0.0%, secondary 0.1%)

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.1%	[0.0%, 0.5%]	43
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	35
Improvements ✅ (primary)	-0.2%	[-0.5%, -0.0%]	8
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	3
All ❌✅ (primary)	0.0%	[-0.5%, 0.5%]	51

Bootstrap: 770.125s -> 769.973s (-0.02%)
Artifact size: 341.42 MiB -> 341.49 MiB (0.02%)

the8472 · 2025-02-09T18:25:41Z

waiting for #135763

Dylan-DPC · 2025-02-25T15:04:31Z

#135763 is now merged so unblocking

rust-bors · 2026-01-14T05:45:25Z

📌 Commit 468eb45 has been approved by nnethercote

It is now in the queue for this repository.

rust-bors · 2026-01-14T19:53:10Z

☀️ Test successful - CI
Approved by: nnethercote
Pushing 86a49fd to main...

github-actions · 2026-01-14T19:55:52Z

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 8c52f73 (parent) -> 86a49fd (this PR)

Test differences

Show 199 test diffs

Stage 1

[codegen] tests/codegen-llvm/lib-optimizations/append-elements.rs: [missing] -> pass (J1)
[codegen] tests/codegen-llvm/lib-optimizations/append-elements.rs: [missing] -> ignore (ignored when the LLVM version 20.1.2 is older than 21.0.0) (J4)

Stage 2

[codegen] tests/codegen-llvm/lib-optimizations/append-elements.rs: [missing] -> ignore (ignored when the LLVM version 20.1.2 is older than 21.0.0) (J0)
[codegen] tests/codegen-llvm/lib-optimizations/append-elements.rs: [missing] -> pass (J2)
[codegen] tests/codegen-llvm/lib-optimizations/append-elements.rs: [missing] -> ignore (ignored when the LLVM version 20.1.8 is older than 21.0.0) (J3)

Additionally, 194 doctest diffs were found. These are ignored, as they are noisy.

Job group index

J0: x86_64-gnu-llvm-20-2
J1: pr-check-2, x86_64-gnu-llvm-21-3
J2: aarch64-apple, aarch64-gnu, aarch64-msvc-1, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-gnu, x86_64-gnu-llvm-21-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1
J3: aarch64-gnu-llvm-20-1
J4: x86_64-gnu-llvm-20-3

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 86a49fd71fecd25b0fd20247db0ba95eeceaba28 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

aarch64-apple: 7730.4s -> 11396.2s (+47.4%)
x86_64-gnu-debug: 6890.0s -> 8410.0s (+22.1%)
dist-i686-mingw: 11143.3s -> 8903.5s (-20.1%)
dist-x86_64-mingw: 10552.7s -> 8861.4s (-16.0%)
pr-check-1: 1526.4s -> 1738.5s (+13.9%)
x86_64-mingw-1: 11594.3s -> 10007.1s (-13.7%)
x86_64-mingw-2: 10805.4s -> 9491.8s (-12.2%)
x86_64-rust-for-linux: 2840.8s -> 2522.7s (-11.2%)
x86_64-gnu-aux: 7384.5s -> 6573.9s (-11.0%)
x86_64-gnu-llvm-21-2: 5697.2s -> 5115.0s (-10.2%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2026-01-14T20:34:55Z

Finished benchmarking commit (86a49fd): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.1%, 1.2%]	10
Regressions ❌ (secondary)	0.2%	[0.1%, 0.9%]	8
Improvements ✅ (primary)	-1.3%	[-2.7%, -0.5%]	3
Improvements ✅ (secondary)	-1.4%	[-4.3%, -0.3%]	6
All ❌✅ (primary)	0.2%	[-2.7%, 1.2%]	13

Max RSS (memory usage)

Results (primary 7.2%)

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

	mean	range	count
Regressions ❌ (primary)	7.2%	[4.1%, 10.2%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	7.2%	[4.1%, 10.2%]	2

Cycles

Results (primary -2.6%, secondary -3.6%)

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)	-2.6%	[-2.6%, -2.6%]	1
Improvements ✅ (secondary)	-3.6%	[-4.4%, -2.9%]	2
All ❌✅ (primary)	-2.6%	[-2.6%, -2.6%]	1

Binary size

Results (primary -0.0%, secondary -0.1%)

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

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.5%]	41
Regressions ❌ (secondary)	0.1%	[0.0%, 0.1%]	8
Improvements ✅ (primary)	-0.4%	[-1.4%, -0.0%]	6
Improvements ✅ (secondary)	-0.8%	[-1.4%, -0.1%]	2
All ❌✅ (primary)	-0.0%	[-1.4%, 0.5%]	47

Bootstrap: 472.286s -> 475.838s (0.75%)
Artifact size: 383.57 MiB -> 383.64 MiB (0.02%)

Zalathar · 2026-01-15T00:45:56Z

The new codegen test appears to have been causing flaky CI failures:

…g#130998" This reverts PR <rust-lang#130998> because the added test seems to be flaky / non-deterministic, and has been failing in unrelated PRs during merge CI.

Revert "avoid phi node for pointers flowing into Vec appends #130998" This reverts PR #130998 because the added test seems to be flaky / non-deterministic, and has been failing in unrelated PRs during merge CI: - #151129 (comment) - #150772 (comment) - #150925 (comment) See also [#t-infra > Tree ops](https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Tree.20ops/with/568111767). > [!NOTE] > > This is a "fallback" PR in case the FileCheck failure isn't obvious (i.e. fix-forward). This PR reverts #130998 wholesale in case the failure is genuine and indicative of a bug in the actual implementation change.

Revert "avoid phi node for pointers flowing into Vec appends rust-lang#130998" This reverts PR rust-lang#130998 because the added test seems to be flaky / non-deterministic, and has been failing in unrelated PRs during merge CI: - rust-lang#151129 (comment) - rust-lang#150772 (comment) - rust-lang#150925 (comment) - rust-lang#151145 (comment) See also [#t-infra > Tree ops](https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Tree.20ops/with/568111767). > [!NOTE] > > This is a "fallback" PR in case the FileCheck failure isn't obvious (i.e. fix-forward). This PR reverts rust-lang#130998 wholesale in case the failure is genuine and indicative of a bug in the actual implementation change.

@ghost

Rollup of 2 pull requests Successful merges: - #151150 (Revert "avoid phi node for pointers flowing into Vec appends #130998") - #151145 (Reduce rustdoc GUI flakyness, take 2) r? @ghost

Rollup merge of #151150 - revert-vec-append, r=Zalathar Revert "avoid phi node for pointers flowing into Vec appends #130998" This reverts PR #130998 because the added test seems to be flaky / non-deterministic, and has been failing in unrelated PRs during merge CI: - #151129 (comment) - #150772 (comment) - #150925 (comment) - #151145 (comment) See also [#t-infra > Tree ops](https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Tree.20ops/with/568111767). > [!NOTE] > > This is a "fallback" PR in case the FileCheck failure isn't obvious (i.e. fix-forward). This PR reverts #130998 wholesale in case the failure is genuine and indicative of a bug in the actual implementation change.

…ng#130998" This reverts commit cd79ff2.

…g#130998" This reverts PR <rust-lang#130998> because the added test seems to be flaky / non-deterministic, and has been failing in unrelated PRs during merge CI.

panstromek · 2026-01-19T15:19:03Z

perf triage:

Some change like this was expected, but this was reverted for non-perf reasons, so I don't think we need to investigate further.

@rustbot label: +perf-regression-triaged

optimize `vec.extend(slice.to_vec())`, take 2 Redoing #130998 It was reverted in #151150 due to flakiness. I have traced this to layout randomization perturbing the test (the failure reproduces locally with layout randomization), which is now excluded.

optimize `vec.extend(slice.to_vec())`, take 2 Redoing rust-lang/rust#130998 It was reverted in rust-lang/rust#151150 due to flakiness. I have traced this to layout randomization perturbing the test (the failure reproduces locally with layout randomization), which is now excluded.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 28, 2024