Remove indirection for `Expr` in `Body` by JonathanBrouwer · Pull Request #164047 · rust-lang/rust

JonathanBrouwer · 2026-10-09T16:19:13Z

While this does not seem to have the measurable perf effect that I hoped, I still think this is nice to merge since it does theoretically remove one pointer dereference anytime a body is visited. I think this has no measurable effect because the Expr in the Body is allocated right next to the Body itself in the arena, so this should always be in L1 cache.

This also works towards making the Body type simpler and possibly removing it, as after rust-lang/compiler-team#1044 it won't contain the Parameters anymore. I think we won't end up removing this type though because BodyId is a useful concept that is inherently linked to bodies.

I might also take a look if there are other places in the compiler where a similar optimization might have measurable effect.

r? @Kivooeo

JonathanBrouwer · 2026-10-09T16:19:22Z

@bors try @rust-timer queue

[PERF] Remove indirection for `Expr` in `Body`

rust-bors · 2026-10-09T17:19:03Z

☀️ Try build successful (CI)
Build commit: 78c5042 (78c5042a8ba3840371f49ff75667c5652aa290fe)
Base parent: 76c9095 (76c90957b7e422c4b9c45192b0197214d7de5a54)

rust-timer · 2026-10-09T20:19:56Z

Finished benchmarking commit (78c5042): 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)

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

Cycles

Results (primary -2.2%, secondary 2.0%)

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

Binary size

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

Bootstrap: 490.84s -> 486.162s (-0.95%)
Artifact size: 406.48 MiB -> 406.49 MiB (0.00%)

rustbot · 2026-10-10T15:09:11Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in need_type_info.rs

cc @lcnr

Some changes occurred in coverage instrumentation.

cc @Zalathar

clippy is developed in its own repository. If possible, consider making this change to rust-lang/rust-clippy instead.

cc @rust-lang/clippy

rustbot · 2026-10-10T15:09:50Z

Changes to the size of AST and/or HIR nodes.

cc @nnethercote

Kivooeo · 2026-10-11T13:39:38Z

@bors r+ rollup

rust-bors · 2026-10-11T13:39:42Z

📌 Commit 319d431 has been approved by Kivooeo

It is now in the queue for this repository.

…uwer Rollup of 3 pull requests Successful merges: - #163918 (remove redundant unsafe) - #164047 (Remove indirection for `Expr` in `Body`) - #164154 (rustc-dev-guide subtree update)

@Kivooeo

Rollup merge of #164047 - JonathanBrouwer:expr-body, r=Kivooeo Remove indirection for `Expr` in `Body` While this does not seem to have the measurable perf effect that I hoped, I still think this is nice to merge since it does theoretically remove one pointer dereference anytime a body is visited. I think this has no measurable effect because the `Expr` in the `Body` is allocated right next to the `Body` itself in the arena, so this should always be in L1 cache. This also works towards making the `Body` type simpler and possibly removing it, as after rust-lang/compiler-team#1044 it won't contain the Parameters anymore. I think we won't end up removing this type though because `BodyId` is a useful concept that is inherently linked to bodies. I might also take a look if there are other places in the compiler where a similar optimization might have measurable effect. r? @Kivooeo

nnethercote

Dammit, I did the thing in GitHub where I hit the "Submit review" button at the top, which doesn't actually submit a review. Recreating what I wrote:

I'm wary of optimizations that don't have any measurable effects. Also, previously all Expr fields were references, but now we have one field that's not, which is an inconsistency.

nnethercote · 2026-10-11T02:08:02Z

 hir-stats Block                    288 (NN.N%)             6            48
 hir-stats GenericBound             256 (NN.N%)             4            64
 hir-stats - Trait                    256 (NN.N%)             4
+hir-stats Body                     240 (NN.N%)             3            80


Why does the Body number go up but the Expr number not go down?

The amount of bodies and expressions stayed the same, and expressions stayed the same size. A body is now larger (because it directly contains an expression) so the total is larger.

This is however an artifact of how this test measures, the total size of the arena has in fact gotten smaller, because we removed a reference per body. Perhaps we should include the total arena size in the test, would be interesting

To be clear: Why the total size got larger is that each expression in a Body is being double counted

…uwer Rollup of 3 pull requests Successful merges: - rust-lang/rust#163918 (remove redundant unsafe) - rust-lang/rust#164047 (Remove indirection for `Expr` in `Body`) - rust-lang/rust#164154 (rustc-dev-guide subtree update)

JonathanBrouwer · 2026-10-12T06:39:40Z

I'm wary of optimizations that don't have any measurable effects.

So am I, and I would not have merged this if I believed it makes the code worse.

Also, previously all Expr fields were references, but now we have one field that's not, which is an inconsistency.

Is there anything we're concretely losing from this? I'm happy to revert if so. I'm also happy to revert if you simply feel strongly about this :3

The PR in my intuition makes the code slightly nicer, as it removes an allocation that is not needed, but I'm happy to revert if you wish as my reason for merging this is very subjective

Kivooeo · 2026-10-12T08:20:59Z

well, in case we keep Body as a wrapper, this PR makes sense to me

nnethercote · 2026-10-12T10:33:30Z

This comment explains why the counting changed. Another reason why the inconsistency is a problem.

JonathanBrouwer · 2026-10-12T20:24:21Z

Fyi I'm at Eurorust this week I'll get to reverting this next week. Feel free to do the revert yourself if you'd like to do this quickly :)

JonathanBrouwer · 2026-10-19T13:04:07Z

I've spend a week thinking about this and concluded that what might solve these issues is for Body to be a thin wrapper around &'hir Expr. I think lets first revert this pr and then consider the way forwards after #163758 merges

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 Oct 9, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Oct 9, 2026

Auto merge of #164047 - JonathanBrouwer:expr-body, r=<try>

78c5042

[PERF] Remove indirection for `Expr` in `Body`

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 Oct 9, 2026

JonathanBrouwer changed the title ~~[PERF] Remove indirection for Expr in Body~~ Remove indirection for Expr in Body Oct 10, 2026

JonathanBrouwer marked this pull request as ready for review October 10, 2026 15:09

rustbot assigned Kivooeo Oct 10, 2026

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 10, 2026

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 10, 2026

JonathanBrouwer added 2 commits October 10, 2026 17:09

Remove indirection for Expr in Body

5c1fb39

Fix clippy

319d431

JonathanBrouwer force-pushed the expr-body branch from a756be0 to 319d431 Compare October 10, 2026 15:09

Kivooeo approved these changes Oct 11, 2026

View reviewed changes

rust-bors Bot 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 Oct 11, 2026

JonathanBrouwer mentioned this pull request Oct 11, 2026

Rollup of 3 pull requests #164159

Merged

rust-bors Bot merged commit 8499655 into rust-lang:main Oct 11, 2026
14 checks passed

rustbot added this to the 1.101.0 milestone Oct 11, 2026

nnethercote reviewed Oct 11, 2026

View reviewed changes

github-actions Bot mentioned this pull request Oct 13, 2026

Toolchain upgrade to nightly-2026-10-11 flux-rs/flux#1811

Merged

JonathanBrouwer mentioned this pull request Oct 19, 2026

Revert "Remove indirection for Expr in Body" #164581

Open

Conversation

JonathanBrouwer commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonathanBrouwer commented Oct 9, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented Oct 9, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Oct 9, 2026

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

rustbot commented Oct 10, 2026

Uh oh!

rustbot commented Oct 10, 2026

Uh oh!

Kivooeo commented Oct 11, 2026

Uh oh!

rust-bors Bot commented Oct 11, 2026

Uh oh!

Uh oh!

nnethercote left a comment

Choose a reason for hiding this comment

Uh oh!

nnethercote Oct 11, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

JonathanBrouwer commented Oct 12, 2026

Uh oh!

Kivooeo commented Oct 12, 2026

Uh oh!

nnethercote commented Oct 12, 2026

Uh oh!

JonathanBrouwer commented Oct 12, 2026

Uh oh!

JonathanBrouwer commented Oct 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JonathanBrouwer commented Oct 9, 2026 •

edited

Loading