miri: optimize zeroed alloc by SpecificProtagonist · Pull Request #136035 · rust-lang/rust

SpecificProtagonist · 2025-01-25T01:27:11Z

When allocating zero-initialized memory in MIR interpretation, rustc allocates zeroed memory, marks it as initialized and then re-zeroes it. Remove the last step.

I don't expect this to have much of an effect on performance normally, but in my case in which I'm creating a large allocation via mmap it gets in the way.

rustbot · 2025-01-25T01:27:20Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @wesleywiser (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot · 2025-01-25T01:27:22Z

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @rust-lang/wg-const-eval

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

jieyouxu · 2025-01-25T07:17:49Z

r? miri

SpecificProtagonist · 2025-01-25T09:51:34Z

Sorry, I'm not sure how I closed this – misclick?

Kobzol · 2025-01-25T11:36:51Z

@bors try @rust-timer queue

bors · 2025-01-25T11:38:02Z

⌛ Trying commit bd28faf with merge 837b710...

…c, r=<try> miri: optimize zeroed alloc When allocating zero-initialized memory in MIR interpretation, rustc allocates zeroed memory, marks it as initialized and then re-zeroes it. Remove the last step. I don't expect this to have much of an effect on performance normally, but in my case in which I'm creating a large allocation via mmap miri is unusable without this. There's probably a better way – with less code duplication – to implement this. Maybe adding a zero_init flag to the relevant methods, but then `Allocation::uninit` & co need a new name :)

bors · 2025-01-25T13:29:27Z

☀️ Try build successful - checks-actions
Build commit: 837b710 (837b710e5dd54b53b888f1ab109a3b93efc9a144)

oli-obk · 2025-01-25T14:11:32Z

This is not gonna show up in perf. No code path outside miri is changed

RalfJung · 2025-01-25T14:21:48Z

We should definitely explore ways to do this with less code duplication. :)
Adding a flag sounds like a good idea. The name of the method could just be Allocation::new etc?

rust-timer · 2025-01-25T15:26:43Z

Finished benchmarking commit (837b710): comparison URL.

Overall result: ✅ improvements - 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 is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

Results (primary -2.2%, secondary 2.2%)

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

Cycles

Results (primary -1.6%, secondary -0.2%)

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

Binary size

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

Bootstrap: 771.261s -> 771.165s (-0.01%)
Artifact size: 325.82 MiB -> 325.82 MiB (0.00%)

SpecificProtagonist · 2025-01-25T18:34:42Z

Adding a flag sounds like a good idea. The name of the method could just be Allocation::new etc?

Changed 👍

RalfJung · 2025-01-28T10:53:12Z

        new_size: Size,
        new_align: Align,
        kind: MemoryKind<M::MemoryKind>,
+        init: AllocInit,


Might be good to explain in a doc comment for the fn that this only refers to the new, grown part of the alloc; the carried-over part is as (un)initialized as the old allocation.

oli-obk · 2025-01-28T11:04:08Z

Please squash the commits once the final doc comment has been added

Co-authored-by: Ralf Jung <post@ralfj.de>

oli-obk · 2025-01-28T11:55:27Z

@bors r+

bors · 2025-01-28T11:55:30Z

📌 Commit eee9df4 has been approved by oli-obk

It is now in the queue for this repository.

RalfJung · 2025-01-28T12:18:42Z

Thanks for the PR, this is a nice optimization :)

bors · 2025-01-30T01:27:24Z

⌛ Testing commit eee9df4 with merge 5e55679...

bors · 2025-01-30T04:11:52Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 5e55679 to master...

rust-timer · 2025-01-30T05:58:20Z

Finished benchmarking commit (5e55679): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

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

Cycles

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

Binary size

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

Bootstrap: 775.772s -> 775.404s (-0.05%)
Artifact size: 328.49 MiB -> 328.40 MiB (-0.03%)

rustbot assigned wesleywiser Jan 25, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 25, 2025

This comment has been minimized.

Sign in to view

rustbot assigned oli-obk and unassigned wesleywiser Jan 25, 2025

SpecificProtagonist closed this Jan 25, 2025

SpecificProtagonist reopened this Jan 25, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 25, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 25, 2025

oli-obk reviewed Jan 25, 2025

View reviewed changes

Comment thread compiler/rustc_const_eval/src/interpret/memory.rs Outdated

oli-obk reviewed Jan 25, 2025

View reviewed changes

Comment thread compiler/rustc_middle/src/mir/interpret/allocation.rs Outdated

RalfJung reviewed Jan 28, 2025

View reviewed changes

Comment thread compiler/rustc_const_eval/src/interpret/memory.rs Outdated

RalfJung reviewed Jan 28, 2025

View reviewed changes

miri: optimize zeroed alloc

eee9df4

Co-authored-by: Ralf Jung <post@ralfj.de>

SpecificProtagonist force-pushed the miri-zeroed-alloc branch from ff38a95 to eee9df4 Compare January 28, 2025 11:50

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 28, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 30, 2025

bors merged commit 5e55679 into rust-lang:master Jan 30, 2025

rustbot added this to the 1.86.0 milestone Jan 30, 2025

This was referenced Jan 30, 2025

Mangle rustc_std_internal_symbols functions #127173

Merged

[WIP] Use weak linkage instead of compiler generated shims #134522

Draft

Fix deduplication mismatches in vtables leading to upcasting unsoundness #135318

Merged

Uh oh!

Conversation

SpecificProtagonist commented Jan 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 25, 2025

Uh oh!

rustbot commented Jan 25, 2025

Uh oh!

This comment has been minimized.

jieyouxu commented Jan 25, 2025

Uh oh!

SpecificProtagonist commented Jan 25, 2025

Uh oh!

Kobzol commented Jan 25, 2025

Uh oh!

This comment has been minimized.

bors commented Jan 25, 2025

Uh oh!

bors commented Jan 25, 2025

Uh oh!

This comment has been minimized.

oli-obk commented Jan 25, 2025

Uh oh!

RalfJung commented Jan 25, 2025

Uh oh!

rust-timer commented Jan 25, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

SpecificProtagonist commented Jan 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Jan 28, 2025

Uh oh!

oli-obk commented Jan 28, 2025

Uh oh!

bors commented Jan 28, 2025

Uh oh!

RalfJung commented Jan 28, 2025 via email

Uh oh!

bors commented Jan 30, 2025

Uh oh!

bors commented Jan 30, 2025

Uh oh!

rust-timer commented Jan 30, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

SpecificProtagonist commented Jan 25, 2025 •

edited

Loading