Avoid no-op unlink+link dances in incr comp #128320

saethlin · 2024-07-28T21:06:32Z

Incremental compilation scales quite poorly with the number of CGUs. This PR improves one reason for that.

The incr comp process hard-links all the files from an old session into a new one, then it runs the backend, which may just hard-link the new session files into the output directory. Then codegen hard-links all the output files back to the new session directory.

This PR (perhaps unimaginatively) fixes the silliness that ensues in the last step. The old link_or_copy implementation would be passed pairs of paths which are already the same inode, then it would blindly delete the destination and re-create the hard-link that it just deleted. This PR lets us skip both those operations. We don't skip the other two hard-links.

cargo +stage1 b && touch crates/core/main.rs && strace -cfw -elink,linkat,unlink,unlinkat cargo +stage1 b before and then after on ripgrep-13.0.0:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 52.56    0.024950          25       978       485 unlink
 34.38    0.016318          22       727           linkat
 13.06    0.006200          24       249           unlinkat
------ ----------- ----------- --------- --------- ----------------
100.00    0.047467          24      1954       485 total

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 42.83    0.014521          57       252           unlink
 38.41    0.013021          26       486           linkat
 18.77    0.006362          25       249           unlinkat
------ ----------- ----------- --------- --------- ----------------
100.00    0.033904          34       987           total

This reduces the number of hard-links that are causing perf troubles, noted in #64291 and #137560

saethlin · 2024-07-28T21:14:58Z

@bors try @rust-timer queue

bors · 2024-07-28T21:16:10Z

⌛ Trying commit 223603c with merge 0a89cd2...

Avoid no-op unlink+link dances in incr comp Incremental compilation scales quite poorly with the number of CGUs. This PR improves one reason for that. The incr comp process hard-links all the files from an old session into a new one, then it runs the backend, which may just hard-link the new session files into the output directory. Then codegen hard-links all the output files back to the new session directory. This PR (perhaps unimaginatively) fixes the silliness that ensues in the last step. The old `link_or_copy` implementation would be passed pairs of paths which are already the same inode, then it would blindly delete the destination and re-create the hard-link that it just deleted. This PR lets us skip both those operations. We don't skip the other two hard-links. `cargo +stage1 b && touch crates/core/main.rs && strace -cfw -elink,linkat,unlink,unlinkat cargo +stage1 b` before and then after on `ripgrep-13.0.0`: ``` % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 52.56 0.024950 25 978 485 unlink 34.38 0.016318 22 727 linkat 13.06 0.006200 24 249 unlinkat ------ ----------- ----------- --------- --------- ---------------- 100.00 0.047467 24 1954 485 total ``` ``` % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 42.83 0.014521 57 252 unlink 38.41 0.013021 26 486 linkat 18.77 0.006362 25 249 unlinkat ------ ----------- ----------- --------- --------- ---------------- 100.00 0.033904 34 987 total ``` r? `@ghost`

bors · 2024-07-28T21:28:14Z

💔 Test failed - checks-actions

saethlin · 2024-07-28T22:33:02Z

@bors try @rust-timer queue

bors · 2024-07-28T22:34:13Z

⌛ Trying commit 26319ff with merge a712ff3...

Avoid no-op unlink+link dances in incr comp Incremental compilation scales quite poorly with the number of CGUs. This PR improves one reason for that. The incr comp process hard-links all the files from an old session into a new one, then it runs the backend, which may just hard-link the new session files into the output directory. Then codegen hard-links all the output files back to the new session directory. This PR (perhaps unimaginatively) fixes the silliness that ensues in the last step. The old `link_or_copy` implementation would be passed pairs of paths which are already the same inode, then it would blindly delete the destination and re-create the hard-link that it just deleted. This PR lets us skip both those operations. We don't skip the other two hard-links. `cargo +stage1 b && touch crates/core/main.rs && strace -cfw -elink,linkat,unlink,unlinkat cargo +stage1 b` before and then after on `ripgrep-13.0.0`: ``` % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 52.56 0.024950 25 978 485 unlink 34.38 0.016318 22 727 linkat 13.06 0.006200 24 249 unlinkat ------ ----------- ----------- --------- --------- ---------------- 100.00 0.047467 24 1954 485 total ``` ``` % time seconds usecs/call calls errors syscall ------ ----------- ----------- --------- --------- ---------------- 42.83 0.014521 57 252 unlink 38.41 0.013021 26 486 linkat 18.77 0.006362 25 249 unlinkat ------ ----------- ----------- --------- --------- ---------------- 100.00 0.033904 34 987 total ``` r? `@ghost`

bors · 2024-07-29T00:16:46Z

☀️ Try build successful - checks-actions
Build commit: a712ff3 (a712ff31daae44e2f087c73598a2922156aae429)

rust-timer · 2024-07-29T01:33:15Z

Finished benchmarking commit (a712ff3): comparison URL.

Overall result: no relevant changes - no 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.

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

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (primary 1.0%, secondary -3.7%)

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

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 767.825s -> 768.215s (0.05%)
Artifact size: 331.40 MiB -> 331.36 MiB (-0.01%)

saethlin · 2024-11-22T04:32:33Z

@bors try @rust-timer queue

saethlin · 2024-11-22T05:57:22Z

@bors try @rust-timer queue

bors · 2024-11-22T05:58:38Z

⌛ Trying commit 6f5792c with merge 3b0f3ea...

rustbot · 2025-02-24T23:34:49Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

saethlin · 2025-02-24T23:34:53Z

r? compiler

the8472 · 2025-02-25T00:29:58Z

Another option would be comparing the dev/ino (or the windows equivalents) of the source and destination file and skip the linking if they're already the same.

saethlin · 2025-02-25T00:38:26Z

The implementation in this PR strictly reduces the amount we interact with the filesystem by using information that's already trivially available to the compiler. Apart from redesigning the incremental compilation CGU reuse system, I don't see any alternative to this optimization that can compete with it for efficiency.

It might be good to also not re-hardlink files that are already hardlinked, but on Linux I'm not sure that is an improvement.

the8472 · 2025-02-25T00:45:29Z

Yeah I was thinking of the linked issues that were about hardlink slowness on macos and windows.

saethlin · 2025-02-25T00:49:49Z

Somebody drive-by reviewed this and made a suggestion that seems to work but now I don't even know how to respond to them.

Please. Do not delete your own comments.

compiler/rustc_codegen_ssa/src/back/write.rs

bjorn3 · 2025-02-26T09:19:02Z

compiler/rustc_codegen_cranelift/src/driver/aot.rs

@@ -487,6 +503,7 @@ fn reuse_workproduct_for_cgu(
            bytecode: None,
            assembly: None,
            llvm_ir: None,
+            links_from_incr_cache: links_from_incr_cache.clone(),


I think both CompiledModule should use a separate links_from_incr_cache.

Oh 🤦 I see now

bjorn3 · 2025-02-27T10:37:46Z

cg_clif changes LGTM

compiler-errors · 2025-03-21T16:44:58Z

@bors r+

bors · 2025-03-21T16:45:01Z

📌 Commit e21502c has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2025-03-21T21:03:53Z

⌛ Testing commit e21502c with merge 48b36c9...

bors · 2025-03-22T00:09:27Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 48b36c9 to master...

github-actions · 2025-03-22T00:10: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 be73c1f (parent) -> 48b36c9 (this PR)

Test differences

No test diffs found

rust-timer · 2025-03-22T01:26:33Z

Finished benchmarking commit (48b36c9): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -7.5%, secondary 3.9%)

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

Cycles

Results (secondary 7.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
Regressions ❌ (secondary)	7.1%	[3.1%, 9.7%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 775.146s -> 775.347s (0.03%)
Artifact size: 365.54 MiB -> 365.55 MiB (0.00%)

rustbot added S-waiting-on-review T-compiler labels Jul 28, 2024

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf label Jul 28, 2024

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author and removed S-waiting-on-review labels Jul 28, 2024

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf label Jul 29, 2024

saethlin force-pushed the link-me-maybe branch from 26319ff to 3d55891 Compare August 16, 2024 16:07

saethlin force-pushed the link-me-maybe branch from 3d55891 to 6f5792c Compare August 23, 2024 22:11

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf label Nov 22, 2024

saethlin closed this Nov 22, 2024

saethlin reopened this Nov 22, 2024

This comment has been minimized.

Sign in to view

rustbot assigned compiler-errors Feb 24, 2025

saethlin mentioned this pull request Feb 24, 2025

Incremental compilation on Windows is slow because of hard links #137560

Open

Avoid no-op unlink+link dances in incr comp

cae7c76

saethlin force-pushed the link-me-maybe branch from 6244f8d to cae7c76 Compare February 25, 2025 00:47

bjorn3 reviewed Feb 25, 2025

View reviewed changes

compiler/rustc_codegen_ssa/src/back/write.rs Show resolved Hide resolved

bjorn3 reviewed Feb 26, 2025

View reviewed changes

saethlin force-pushed the link-me-maybe branch from 1932ee5 to 05cf391 Compare February 26, 2025 23:28

This comment has been minimized.

Sign in to view

Fill out links_from_incr_cache in cg_clif

e21502c

saethlin force-pushed the link-me-maybe branch from 05cf391 to e21502c Compare February 27, 2025 01:02

jkelleyrtp mentioned this pull request Mar 19, 2025

Binary patching rust hot-reloading, sub-second rebuilds, independent server/client hot-reload DioxusLabs/dioxus#3797

Draft

13 tasks

bors added S-waiting-on-bors and removed S-waiting-on-review labels Mar 21, 2025

bors mentioned this pull request Mar 21, 2025

Rollup of 8 pull requests #138803

Closed

bors added the merged-by-bors label Mar 22, 2025

bors merged commit 48b36c9 into rust-lang:master Mar 22, 2025
7 checks passed

rustbot added this to the 1.87.0 milestone Mar 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid no-op unlink+link dances in incr comp #128320

Avoid no-op unlink+link dances in incr comp #128320

saethlin commented Jul 28, 2024 •

edited

Loading

saethlin commented Jul 28, 2024

This comment has been minimized.

bors commented Jul 28, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Jul 28, 2024

saethlin commented Jul 28, 2024

This comment has been minimized.

bors commented Jul 28, 2024

bors commented Jul 29, 2024

This comment has been minimized.

rust-timer commented Jul 29, 2024

saethlin commented Nov 22, 2024

This comment has been minimized.

saethlin commented Nov 22, 2024

This comment has been minimized.

bors commented Nov 22, 2024

rustbot commented Feb 24, 2025

saethlin commented Feb 24, 2025

the8472 commented Feb 25, 2025

saethlin commented Feb 25, 2025

the8472 commented Feb 25, 2025

saethlin commented Feb 25, 2025

bjorn3 Feb 26, 2025

saethlin Feb 26, 2025

This comment has been minimized.

bjorn3 commented Feb 27, 2025

compiler-errors commented Mar 21, 2025

bors commented Mar 21, 2025

bors commented Mar 21, 2025

bors commented Mar 22, 2025

github-actions bot commented Mar 22, 2025

rust-timer commented Mar 22, 2025

Avoid no-op unlink+link dances in incr comp #128320

Avoid no-op unlink+link dances in incr comp #128320

Conversation

saethlin commented Jul 28, 2024 • edited Loading

saethlin commented Jul 28, 2024

This comment has been minimized.

bors commented Jul 28, 2024

This comment has been minimized.

This comment has been minimized.

bors commented Jul 28, 2024

saethlin commented Jul 28, 2024

This comment has been minimized.

bors commented Jul 28, 2024

bors commented Jul 29, 2024

This comment has been minimized.

rust-timer commented Jul 29, 2024

Overall result: no relevant changes - no action needed

saethlin commented Nov 22, 2024

This comment has been minimized.

saethlin commented Nov 22, 2024

This comment has been minimized.

bors commented Nov 22, 2024

rustbot commented Feb 24, 2025

saethlin commented Feb 24, 2025

the8472 commented Feb 25, 2025

saethlin commented Feb 25, 2025

the8472 commented Feb 25, 2025

saethlin commented Feb 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

bjorn3 commented Feb 27, 2025

compiler-errors commented Mar 21, 2025

bors commented Mar 21, 2025

bors commented Mar 21, 2025

bors commented Mar 22, 2025

github-actions bot commented Mar 22, 2025

Test differences

rust-timer commented Mar 22, 2025

Overall result: no relevant changes - no action needed

saethlin commented Jul 28, 2024 •

edited

Loading