Move std::io::Write to core::io

[bushrat011899]

View all comments

ACP: rust-lang/libs-team#755
Tracking issue: #154046
Split From: #156527
Blocked On: #158537
Blocked On: #158540

Description

Moves std::io::Write to core::io. This is effectively a direct cut and paste, but with a large diff due to how many implementations need to be moved into alloc and core. Blocked on #158537 and #158540.

---

Notes

• No AI tooling of any kind was used during the creation of this PR.
• Please see Tracking Issue for alloc::io and core::io #154046 (comment) for a review order and broader context for this PR.

[rustbot]

r? @Darksonn

rustbot has assigned @Darksonn.
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: @ChrisDenton, libs
• @ChrisDenton, libs expanded to 13 candidates
• Random selection from 6 candidates

[bushrat011899]

@rustbot blocked

[bushrat011899]

@rustbot ready
@clarfonthey This is the next PR in my chain if you're interested in reviewing this one too! I'm not assigning you just in case you don't want to take it on.

[clarfonthey]

r? clarfonthey

[JonathanBrouwer]

@rust-timer build 0ffdcdb

[rust-timer]

Missing artifact for sha 0ffdcdb2b658591abfb86086a931c3cb8227c0ce (https://ci-artifacts.rust-lang.org/rustc-builds/0ffdcdb2b658591abfb86086a931c3cb8227c0ce/rustc-nightly-x86_64-unknown-linux-gnu.tar.xz); not built yet, try again later.

[JonathanBrouwer]

@rust-timer build 0ffdcdb

[rust-timer]

Finished benchmarking commit (0ffdcdb): 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 @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

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.5%	[0.1%, 0.7%]	3
Regressions ❌ (secondary)	0.7%	[0.0%, 1.3%]	2
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[-0.4%, 0.7%]	4

Max RSS (memory usage)

Results (primary -1.0%, secondary 0.3%)

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

	mean	range	count
Regressions ❌ (primary)	3.5%	[3.5%, 3.5%]	1
Regressions ❌ (secondary)	8.6%	[8.5%, 8.6%]	2
Improvements ✅ (primary)	-2.5%	[-3.4%, -1.9%]	3
Improvements ✅ (secondary)	-5.2%	[-8.5%, -2.1%]	3
All ❌✅ (primary)	-1.0%	[-3.4%, 3.5%]	4

Cycles

Results (primary -3.2%, secondary -4.4%)

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)	-3.2%	[-3.2%, -3.2%]	1
Improvements ✅ (secondary)	-4.4%	[-6.7%, -2.1%]	2
All ❌✅ (primary)	-3.2%	[-3.2%, -3.2%]	1

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.0%	[0.0%, 0.0%]	4
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	2
Improvements ✅ (primary)	-0.9%	[-0.9%, -0.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.2%	[-0.9%, 0.0%]	5

Bootstrap: 490.768s -> 490.566s (-0.04%)
Artifact size: 389.74 MiB -> 389.77 MiB (0.01%)

[JonathanBrouwer]

Regressions seem real
@bushrat011899

[bushrat011899]

@JonathanBrouwer interesting! This PR only moves 1 trait, a handful of private functions, and said trait's libcore and liballoc implementations. There's no changes to how the trait works or is implemented, so I can only assume the compiler itself just has to work harder when a trait is referenced across multiple crates? It might be worth checking #158544, which moves Read. If there's a compile time regression for Write, I'd expect to see the same thing in moving Read. The only difference is Read is only being moved to liballoc, so it'd be interesting to confirm that the same regression is seen.

[clarfonthey]

Entirely possible that there are just some functions that should have been marked inline but weren't.

[clarfonthey]

Not sure how perf triage likes to handle this, but I was planning to block merging further core::io changes until we can get some guarantee the perf doesn't regress, and we've fixed this.

I mention inlining since it's an obvious cross-crate thing that might apply here, but I don't actually know the cause and haven't properly looked.

[JonathanBrouwer]

It would be nice if you could look into the regressions before continuing so we can at least understand where they come from. They're relatively small though so if they're discovered not to be solvable they shouldn't block progress imo

[clarfonthey]

To be clear, "not regress" here mostly means that if it turns out to be a very weird rabbit hole that we can't fix, we can just let it be, but if it gets fixed by tweaking a few inline attributes or rearranging some code, we should definitely make those fixes first.